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

Add communication error in status function. #5

Merged
merged 7 commits into from Jan 9, 2019
Merged

Add communication error in status function. #5

merged 7 commits into from Jan 9, 2019

Conversation

d21d3q
Copy link
Contributor

@d21d3q d21d3q commented Jan 9, 2019

having status function that return True in case no response from printer might be misleading. In library it is written that it might be usefull if there is no RX connected. This commit makes it backward compatible, but IMO it should rise error by defualt, and be able to suppress... but if user can suppress error (because RX is not connected) then he don't care about printer status, so maybe it would be better to set all fields to False by default?

@BoboTiG
Copy link
Owner

BoboTiG commented Jan 9, 2019

Thank you very much @d21d3q!
Think to fix the failing test about the method signature you changed ;)

@BoboTiG
Copy link
Owner

BoboTiG commented Jan 9, 2019

Also add yourself to CONTRIBUTORS :)

@BoboTiG
Copy link
Owner

BoboTiG commented Jan 9, 2019

Do you want to add yourself to the CONTRIBUTORS file?

@d21d3q
Copy link
Contributor Author

d21d3q commented Jan 9, 2019

I think I can live without it ;)

@BoboTiG BoboTiG merged commit d486f57 into BoboTiG:master Jan 9, 2019
@BoboTiG
Copy link
Owner

BoboTiG commented Jan 9, 2019

Thanks!

@BoboTiG
Copy link
Owner

BoboTiG commented Jan 9, 2019

FI I have some ongoing work and will publish a new version on PyPi very soon.

@d21d3q
Copy link
Contributor Author

d21d3q commented Jan 9, 2019 via email

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

2 participants