-
Notifications
You must be signed in to change notification settings - Fork 53
Allow uart to go ~100Hz versus 10Hz #50
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
Conversation
Why have the sleep at all? Why not just wait for two bytes? |
@tannewt
And it didn't work. It kept getting caught for some reason. |
You may need to add a timeout so that if it doesn't work you can escape the loop. Basically track how long has passed since you started and exit the loop if it is too long. |
Ah, ok, that makes sense. I'll do that. |
@tannewt So, I added the timeout, and I also just added a copy of _read_register that only gets run if there would have been a UART read error. Just adding that bit of recursion has made the UART mode way more stable. |
It is slightly janky, and if you've got a cleaner way to do it, let me know because I definitely think this could be a lot cleaner. |
adafruit_bno055.py
Outdated
resp = self._uart.read(self._uart.in_waiting) | ||
|
||
if resp[0] != 0xBB: # Recursion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not recursion because recursion would call _read_register. Why did you add this? It wasn't needed before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial plan was to actually do that, but I couldn't think of a good way to add in an exit condition so it didn't just keep going until it reached the recursion limit, and I guess I forgot to remove the comment.
It's needed because the, when running in UART mode, even in the version that's currently on master, it raises a UART read error every 3 or 4 reads when you're trying to use the simpletest. I think that before it may have mostly been tested by getting a single value, such as acceleration, at a time. That being said, even though I know the majority of people aren't using this with UART, I am quite surprised that I haven't seen anyone else encountering that issue, so it may be worthwhile for some other people with the sensor to do some testing to ensure that issue doesn't only affect me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the errors go away if you increase the timeout? If you really need it, I think you could just have an outer loop and count how many tries you want to do before raising an error. No need to copy things a second time.
adafruit_bno055.py
Outdated
resp = self._uart.read(self._uart.in_waiting) | ||
|
||
if resp[0] != 0xBB: # Recursion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the errors go away if you increase the timeout? If you really need it, I think you could just have an outer loop and count how many tries you want to do before raising an error. No need to copy things a second time.
Ok, I think I'll do that. They don't. It seems it just gets stuck and the only way to get it unstuck (without using a try/except in code.py) is to re-request the data from the sensor. |
@tannewt Yeah, this works way better now. I felt confident enough in it to lower the timeout time to 0.1 seconds from 0.25 and I let it run for a while and it hasn't given me any issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor things. Looks good otherwise. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you!
Updating https://github.com/adafruit/Adafruit_CircuitPython_BNO055 to 5.0.2 from 5.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_BNO055#50 from adafruit/uart_speed Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_BroadcastNet to 0.10.1 from 0.10.0: > Merge pull request adafruit/Adafruit_CircuitPython_BLE_BroadcastNet#12 from adafruit/discord-fix > Fixed discord invite link Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 0.11.4 from 0.11.3: > Fix README rendering Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Added the following libraries: Adafruit_CircuitPython_ImageLoad
So I tried a bunch of different ways involving checking how many bytes were in waiting, but in the end, just making the time.sleep smaller and adding a try/except loop was the fastest and most stable way to do it.