Several fixes / improvements

[Boscop]

Several fixes / improvements:

• On windows, midiInOpen() only succeeds when callback is extern "system", so I changed it to that. Now it works.
• Improved error handling when getting port name: Under some conditions (such as when MidiHub is installed but not running), getting the portname can fail, but only for the MidiHub port! Before it crashed because of assert, now it handles this case gracefully. The reason this is necessary is because often you want to list all input or output port names, allowing the user to choose one. This should just ignore the port names that cannot be queried, for example:

		let midi_out = try!(MidiOutput::new(""));
		let port_out = try!((|| {
			for i in 0..midi_out.port_count() {
				if let Ok(name) = midi_out.port_name(i) {
					if name == cfg.port_name {
						return Some(i);
					}
				}
			}
			return None;
		})().ok_or(r#"midi output port not found"#));

• A similar issue is: port names should be reported as they appear in the OS, without any added number! The UI can add a number if that's wanted. The reason, seen above, is that the port can be given by name in a config file that the app stores. Port numbers change when devices get added or removed, port names stay the same. So this method of finding the port name by name rather than number between app sessions is more robust, but it requires that the api returns the name without added number!
• Some small things like sleep(Duration::from_millis(1)); instead of sleep_ms and extend_from_slice() instead of push_all().
• Most of the time when you want to send midi messages, they are MIDI Short Messages, being expressible in 3 bytes. It is wasteful to allocate a vector and iterate through it and deallocate it for each short message, especially when you are sending a lot of MIDI messages, so I added a function to send MIDI Short Messages expressed in a u32, that can send them directly without any conversion necessary.

pub fn send_short_message(&mut self, message: u32) -> Result<(), SendError>

Most likely in your app you will have a type for MIDI Short Messages anyway.
It will have a method to_u32():

#[derive(Clone, Copy, Debug, PartialEq)]
pub struct ShortMsg {
	pub data0: u8,
	pub data1: u8,
	pub data2: u8,
}
impl ShortMsg {
	pub fn status(&self) -> Status { // Status and masks are from the rimd crate
		Status::from_u8(self.data0 & STATUS_MASK).unwrap()
	}
	pub fn channel(&self) -> u8 {
		self.data0 & CHANNEL_MASK
	}
	pub fn to_u32(&self) -> u32 {
		((((self.data2 as u32) << 16) & 0xFF0000) |
		  (((self.data1 as u32) << 8) & 0xFF00) |
		  ((self.data0 as u32) & 0xFF)) as u32
	}
	...
	// different constructors here for the different types of short msgs
}

Since I'm on windows, I only implemented this for windows, but I'm sure it's easy to add it for other OSes, too.

Btw, this crate goes well together with the rimd crate :)
Maybe it would make sense to unify them into one project in the future..

[Boddlnagg]

First, thanks for the PR! It's a little hard for me to comment on this since so many unrelated changes are thrown together ... could you rebase these changes on master an also merge those commits that just change or clean a previous change? Or even better, submit unrelated parts as different, smaller PRs? That would make it a lot easier for me to review ...

Still, I can provide some comments:

On windows, midiInOpen() only succeeds when callback is extern "system", so I changed it to that. Now it works.

Yeah, you're right. system is equivalent to C on 64-bit Windows (that's why it worked for me), but on 32-bit stdcall must be used instead, whereas system does the right thing on 64- and 32-bit.

Improved error handling when getting port name

Yep, but could you change the error condition from result == MMSYSERR_ERROR to result != MMSYSERR_NOERROR ? Other types of errors might occur (see https://msdn.microsoft.com/en-us/library/windows/desktop/dd798469(v=vs.85).aspx).

A similar issue is: port names should be reported as they appear in the OS, without any added number!

This is based on code from rtmidi, which midir is based on. I'm not sure why they added it, but I guess the problem is that you will get the same "identifier" for multiple devices with the same name (I don't know how often that happens). The whole port-selection API is a bit suboptimal, because it doesn't map well between platforms (especially ALSA needs careful treatment here). See also thestk/rtmidi#15. And there is the (unsolvable?) problem that the port count returned from port_count() might be invalid once you call port_name(), because a device was removed in the meantime.

On the other hand, I guess that your change is not too bad and a user could still add the number manually if unique names are required, so I'm inclined to accept it.

Some small things like sleep(Duration::from_millis(1)); instead of sleep_ms and extend_from_slice() instead of push_all().

Both of these are already in master

Most of the time when you want to send midi messages, they are MIDI Short Messages, being expressible in 3 bytes.

Yes, this is a little inefficient, but if such a thing is added, it must be added to all backends. I have to look into the other backends to see if functions similar to midiOutShortMsg are available, because if not, such a backend might need to allocate, which would make an implementation of send_short_message (which sounds "more efficient") actually less efficient on some platforms. I'm hesitant because it's also not available in rtmidi.

Also, the MidiShortMessage struct should not be part of midir in its current form, because, as I say in the README, a higher-level API to assemble MIDI messages is not yet included and, when it will be included (which is totally possible at some point), should be carefully designed.

Anyway, thanks for your interest in midir and your contributions! It would be great if you could just factor out the first three changes as separate PRs, based on the current state of the master branch. 👍

And thanks for the hint about rimd ... I don't think that unifying is the right way forward, but we should definitely try to make the two crates work together as seamlessly as possible.

[Boscop]

Thanks, so should I do git rebase or git reset, and then create 3 branches? Or how would you do it?
(Btw, I wasn't suggesting to add MidiShortMessage to midir. I had the type added in my fork but removed it from the fork and put it in my app project because it depends on the Status and masks from rimd.)

[Boddlnagg]

I would start from master and then create three different branches with the small changes. They shouldn't interfere with each other, so there should be no merge conflicts.

[Boscop]

Ok, I'm on it! Apparently I can't create a new fork, so I have to delete my old fork and fork again from master?

[Boddlnagg]

You don't need to fork again. You created this PR from your master branch, but you can just create another branch in your fork, reset that to my master branch (if Boddlnagg/midir is added as a remote repository) and work from there.

[Boddlnagg]

Because this PR was not updated, I have extracted the changes that were still revelant in 197e4c2, thus superseding this PR.