Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow reading zero-bytes (0x00) using getbyte. #20

Merged
merged 1 commit into from
Jan 29, 2015

Conversation

donv
Copy link
Contributor

@donv donv commented Jan 16, 2015

We experienced getting nil instead of a zero byte when reading using getbyte.

Can this make the 0.2.2 release? 😄

@donv
Copy link
Contributor Author

donv commented Jan 16, 2015

Hi!

I see the tests fail in travis, but they succeed for me locally on Linux and OS X. Do they pass for you?

@donv
Copy link
Contributor Author

donv commented Jan 19, 2015

Hi!

OK, travis build is green! Any chance this can be merged and released?

@zankich
Copy link
Contributor

zankich commented Jan 19, 2015

@donv ah yes so your source is sending you a null character, but rubyserial is passing it along as a ruby nil and not the 0x00. A similar change is required for the windows implementation so that we have consistent behavior between both platforms https://github.com/hybridgroup/rubyserial/blob/master/lib/rubyserial/windows.rb#L64

@donv
Copy link
Contributor Author

donv commented Jan 19, 2015

@zankich yes. Do we need to fix it for windows before merging this PR?

@zankich
Copy link
Contributor

zankich commented Jan 20, 2015

@donv yes if you wouldn't mind making the change and then we can test to make sure the behavior is correct, thanks!

@donv
Copy link
Contributor Author

donv commented Jan 29, 2015

Hi @zankich !

I do not have a Windows development environment, and I don't want to invest in that now. It would be great if you could add the fix for non-windows users, and a contributor on windows could fix it for windows.

Is that doable?

@zankich
Copy link
Contributor

zankich commented Jan 29, 2015

@donv yes I'll merge this now and get the windows fix in place, thanks for the pull request!

zankich added a commit that referenced this pull request Jan 29, 2015
Allow reading zero-bytes (0x00) using getbyte.
@zankich zankich merged commit cc8736f into hybridgroup:master Jan 29, 2015
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a7b942e on donv:master into * on hybridgroup:master*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants