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

Fix for Issue #27 #26

Open
wants to merge 2 commits into
base: master
from

Conversation

@devoh747
Copy link

devoh747 commented Oct 9, 2019

added "try" to capture the errors and to retry getting a good packet.

added "try"
try:
packet = Packet.from_stream(uart_server)
except (ValueError, OSError):
pass
if isinstance(packet, ColorPacket):

This comment has been minimized.

Copy link
@jerryneedell

jerryneedell Oct 9, 2019

Contributor

Would it be better to put lines 24-26 after line 21 in the successful try so they are not executed if the error is thrown?

This comment has been minimized.

Copy link
@jerryneedell

jerryneedell Oct 9, 2019

Contributor

Also do you want to to "fail silently" Should the error be printed?

This comment has been minimized.

Copy link
@dhalbert

dhalbert Oct 9, 2019

Collaborator

Usually this fails because the connection has been broken after the while test for connected. So it would be best to fail completely and then go back and test for connected again.

This comment has been minimized.

Copy link
@devoh747

devoh747 Oct 10, 2019

Author

Would it be better to put lines 24-26 after line 21 in the successful try so they are not executed if the error is thrown?

According to https://mail.python.org/pipermail/tutor/2005-July/040116.html you can use a try except continue inside a loop and it will go to the next iteration of the loop. I found this to be true in my fix as it would catch the bad packet and immediately grab a new packet without hitting line 24 and erroring out.

This comment has been minimized.

Copy link
@jerryneedell

jerryneedell Oct 10, 2019

Contributor

Is pass the same as continue in this case?

This comment has been minimized.

Copy link
@devoh747

devoh747 Oct 10, 2019

Author

Is pass the same as continue in this case?

According to
https://stackoverflow.com/questions/9483979/is-there-a-difference-between-continue-and-pass-in-a-for-loop-in-python they are not. I should have used continue in my PR as that would force the loop to start over.

This comment has been minimized.

Copy link
@devoh747

devoh747 Oct 10, 2019

Author

@dhalbert : Should I correct my PR to use continue, or do you just want me to close it and take a different approach?

This comment has been minimized.

Copy link
@dhalbert

dhalbert Oct 10, 2019

Collaborator

continue makes sense.

This comment has been minimized.

Copy link
@devoh747

devoh747 Oct 10, 2019

Author

Usually this fails because the connection has been broken after the while test for connected. So it would be best to fail completely and then go back and test for connected again.

I was able to get the fail to happen after pressing many different buttons where it was obvious it couldn't keep up with the input. So in my case it should still have been connected.. but not sure how to test that.. hmmm I could perhaps in my except clause check the connection status.

This comment has been minimized.

Copy link
@devoh747

devoh747 Oct 10, 2019

Author

I changed my "fix" which is really just a workaround as @jerryneedell and I discussed in discord. I am now just catching the ValueError, printing out that I caught it and "continue"ing on to the next loop iteration. I can really only get this error in more complicated animations where you can issue more commands from the phone app than you can process in your program. I still think it's helpful as an example in the code on what you can do as a work around if your packet generates an error and does no harm if it doesn't.

If you use ble_color_picker.py and make more complicated animations like https://github.com/gallaugher/PythonSmartTie  you can issue packets faster than you can process them and get a ValueError. I use a try to catch it and print out a message but continue to the next loop iteration which grabs the next packet. This fixes crashes/hangs in John's code.. and I think is helpful in this example code.
@devoh747 devoh747 changed the title Fix for Issue #24 Fix for Issue #27 Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.