hparra / ruby-serialport

ruby-serialport is a Ruby library that provides a class for using RS-232 serial ports
http://rubygems.org/gems/serialport
GNU General Public License v2.0
246 stars 58 forks source link

Can't open COM port greater than COM9 #12

Closed corrdump closed 14 years ago

corrdump commented 14 years ago

For windows, a different string must be used for the port parameter in the open() command for COM ports greater than COM9. The sp_create_impl() function does not take this into account, so trying to open COM10 always fails.

COM ports with higher numbers should be opened as .\COMxx, where xx is a port number greater than 9

For example: fd = open("\\.\COM10", O_BINARY | O_RDWR);

I'm running serialport-1.0.1 and ruby 1.8

thanks Michael

hparra commented 14 years ago

Correct. See http://support.microsoft.com/default.aspx?scid=kb;EN-US;q115831

According to the article, COMs 1-9 can also be opened in this fashion, so we can have one method of opening the ports within sp_create_impl().

If I make the changes can someone help with testing?

corrdump commented 14 years ago

Sure. I'd be happy to help

hparra commented 14 years ago

Coincidently, this is where the random printfs #11 are.

hparra commented 14 years ago

@corrdump A little confused. I'm assuming that using "\.\COM10" already works? The code seems to imply this. It is annoying though. Do you wish to be able to just "COM10" or "10" as usual?

corrdump commented 14 years ago

I'll leave the final syntax up to you (but I prefer the latter option). I made a quick mod to my copy of win_serialport_impl.c, so I could get my project running - but it's specific to how I use the serialport gem. So, you may want to add more robust code.

I am only supporting the option to pass "COMxx" string, not the port index number, "xx". So, I added:

sprintf(portStr,"\\.\%s",port); printf("SerialPort => %s\n", portStr); fd = open(portStr, O_BINARY | O_RDWR);

I didn't fix the T_FIXNUM case, as it involved having to modify 'char *ports[]' array, which was more involved than I needed to get my ruby class working

hparra commented 14 years ago

The T_FIXNUM is the easier case. That array seemed unnecessary due to my first comment. With the T_STRING case we want to be careful not to break the interface that exists, as some people are already using the "\.\COM"-style string. I added a not so glamourous check...

I'm not using a windows or an IDE so this obviously needs testing (I don't even know if it compiles!). If I push a branch can you pull it and compile the gem, or do you want a patch?

hparra commented 14 years ago

Please see and test: http://github.com/hparra/ruby-serialport/commit/cc3dfd3cd9faaa3460b78e5c83451b4d97e326ee

corrdump commented 14 years ago

I tested your commit and had to make two small changes to get it to work. See http://github.com/hparra/ruby-serialport/commit/cc3dfd3cd9faaa3460b78e5c83451b4d97e326ee#commitcomment-63491

hparra commented 14 years ago

Please pull new commit. Test 10, "COM10", & "\.\COM10" please. If it's all good I'll release gem as 1.1 since this changes the interface.

corrdump commented 14 years ago

One small change to get it to compile. See http://github.com/hparra/ruby-serialport/commit/62922a13d2cbb6512f8ce9e81d96cbfa1bed78c3#comments

hparra commented 14 years ago

Alright. I changed that but also changed an INT to an UNSIGNED LONG to get rid of warning you mentioned. I mistook another users "looks good" for yours. Hopefully I didn't hose things... X_X

Keeping this open in case there is fallout...

corrdump commented 14 years ago

I compiled your data type change and I don't get the warning any more. But, I don't know how to test this specific change beyond that

corrdump commented 14 years ago

Just checking in to see when this fix will be officially released. I believe you mentioned it would be released as 1.1.0. Is this coming soon?

thanks Michael

hparra commented 14 years ago

Sorry, I released this as 1.0.3 over a month ago. Have you tried it? Does it not work?

corrdump commented 14 years ago

Oops. Didn't see that. I was looking for "1.1.0". Just installed 1.0.3 and it seems to work fine. Thanks!

Michael

hparra commented 14 years ago

Can you do me a favor and confirm it's working in production so I can close this? Thanks.

corrdump commented 14 years ago

I've been using 1.0.3 for the past few days and haven't seen any issues. Thanks