Skip to content

Fix for Issue #27 #26

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.

Sign up for GitHub

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

Closed
devoh747 wants to merge 2 commits into from
Closed

Fix for Issue #27 #26

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
745de98
Fix for Issue #24
Oct 9, 2019
7566483
Update ble_color_picker.py
Oct 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion examples/ble_color_picker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
pass

while uart_server.connected:
packet = Packet.from_stream(uart_server)
try:
packet = Packet.from_stream(uart_server)
except ValueError:
print("Caught a bad packet with a ValueError, discarding and trying again")
continue
if isinstance(packet, ColorPacket):
Copy link
Contributor

@jerryneedell jerryneedell Oct 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@jerryneedell jerryneedell Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@dhalbert dhalbert Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@devoh747 devoh747 Oct 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@jerryneedell jerryneedell Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is pass the same as continue in this case?

Copy link
Author

@devoh747 devoh747 Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@devoh747 devoh747 Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@dhalbert dhalbert Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continue makes sense.

Copy link
Author

@devoh747 devoh747 Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@devoh747 devoh747 Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

print(packet.color)
pixels.fill(packet.color)