search

robohouse-delft / dynamixel2-rs

Rust implementation of the dynamixel protocol 2.0
BSD 2-Clause "Simplified" License
13 stars 4 forks source link

Allow using the broadcast ID with more functions. #10

Closed de-vri-es closed 6 months ago

de-vri-es commented 7 months ago

This PR changes the functions for instructions that do not return any data.

If the motor ID is the broadcast ID, the functions will no longer wait for a response (and then get a timeout). Instead, the functions will return directly after writing the command with a fake response from the broadcast ID.

Additionally (but unrelated), this PR removes the ReadBuffer and WriteBuffer generic arguments from StatusPacket, which should make it a more friendly type to work with. It moves the bookkeeping for used bytes into the Bus struct to allow for this.

Also unrelated, this PR handles full buffers by returning a BufferFullError instead of panicking.

@omelia-iliffe: When you have the time, could you verify that communication is still working with this PR? The changes to StatusPacket are a bit risky considering this kind of bookkeeping is error prone.

wddler commented 6 months ago

I can't say much because I'm not familiar with the project enough. But the PR in general looks complete with comprehensive description, useful comments and passed tests.

omelia-iliffe commented 6 months ago

Sorry its taken me a few days. I've tested all the functions you changed and all immediately return a Response<()> as expected. I confirmed the Dynamixel had also received and processed the broadcast packet as well.

It all looks good to me.

de-vri-es commented 6 months ago

Sorry its taken me a few days. I've tested all the functions you changed and all immediately return a Response<()> as expected. I confirmed the Dynamixel had also received and processed the broadcast packet as well.

It all looks good to me.

Awesome. Thanks for testing!

de-vri-es commented 6 months ago

Released as v0.8.0 :rocket: