Fix status response reading logic #4
Conversation
when using sync_read at high baudrates multiple response packets are read at the same time, and the tailing packets discarded. By checking if a packet is in the buffer before trying to read new bytes this is prevented.
|
Another implementation option is that only the correct and necessary bytes are read from the serial port. This is how Dynamixel's SDK works. |
|
Hey! Thanks for reporting the issue and fixing it! :D I like the approach of this PR. It should be more efficient than limiting the read with The only thing I'm thinking, |
Ah, I see it couldn't panic. But we can remove the entire outer |
|
Sweet, I made that change. |
Good point! The change looks good. For future readers of the code, I also added a code suggestion to explain this with a comment. If you agree with the wording we can apply it and get this merged :) |
Co-authored-by: Maarten de Vries <maarten@de-vri.es>
|
Yup! Looks great! |
|
Released as |
Currently when
read_status_responseis called fromsync_read_cbmultiple packets can be read into the buffer but only one packet is parsed. This leads to the second response packet to be consumed without being parsed.A sync read instruction packet is sent to 2 dynamixels, ID 1 and 2
The expected response data is read into the buffer on the first call to
read_status_response. Note that 2 packets are read, not one.however the current implementation doesn't check if more than one packet was read so it timeouts trying to read a response it has already read.
Moving the read buffer checks, that check if the read buffer has a valid packet, to before the
self.serial_port.readcall prevents this issue. Although the nested if statements may not be the most elegant solution.