ease of use improvements

[omelia-iliffe]

Another week another pull request 🛩️

Couple of things that I needed to change:

Add getters for the internal SerialPort

Useful if you want to use or configure the serial port directly (such as using the SerialPort's set_read_timeout fn)
I have more thoughts about this but don't have the time rn

use AsRef for the data in SyncWriteData

If you have to provide the data as &'a [u8]but have it stored in as a Vec<u8> you have to call as ref on it:

which leads to this mess (sync_write_pos: Vec<SyncWriteData<Vec>)

 self.bus
     .sync_write(
         reg.address,
         reg.length,
         sync_write_pos
             .iter()
             .map(|d| SyncWriteData {
                 motor_id: d.motor_id,
                 data: d.data.as_ref(),
             })
             .collect::<Vec<_>>(),
     )

changing the signature to include AsRef leads to cleaner user code:

self.bus.sync_write(reg.address, reg.length, data)

A quick look at BulkWriteData shows that the data is already typed: AsRef but implementation doesn't use iterators. I could update the bulk_write fn to use iterators. Making both the function impls similar

[de-vri-es]

Looks good! Thanks :D

Regarding the timeouts, I'm also working on a PR to remove the manual read_timeout parameter and instead compute it from the expected response size and the baud rate for each read.

In that PR, I'm also adding calls to set_read_timeout. I'll update the documentation in that PR though, to add a note about it in the getter for the serial port.

[de-vri-es]

Nice addition!

Not backwards compatible, but that's okay. I have more incompatible changes coming up :p

[de-vri-es]

A quick look at BulkWriteData shows that the data is already typed: AsRef but implementation doesn't use iterators. I could update the bulk_write fn to use iterators. Making both the function impls similar

This sounds good too!

[omelia-iliffe]

sorry have been very inactive. Busy with install of our project. Will try find some more time this week or if you have time feel free to start/do it all :)

[de-vri-es]

A quick look at BulkWriteData shows that the data is already typed: AsRef but implementation doesn't use iterators. I could update the bulk_write fn to use iterators. Making both the function impls similar

This sounds good too!

Ah, I tried to implement this, but I figured out why this isn't the case currently: we need to calculate the number of parameters first, before building the instruction message. This means we need to loop over the writes twice.

The Iterator trait isn't great for that. We could demand Iterator + Clone, but some iterator implementations might clone their data.

There are ways around it, but all of them require allocations. So I figured it's ok to limit the input to a slice and make the caller responsible for the allocation.

But I'm always open to suggestions to improve this :)

[de-vri-es]

sorry have been very inactive. Busy with install of our project. Will try find some more time this week or if you have time feel free to start/do it all :)

No problem. I'm in the middle of moving and renovating my new place, so I'm also quite busy. But I think I can find some time for this crate :)

[de-vri-es]

Ah, I tried to implement this, but I figured out why this isn't the case currently: we need to calculate the number of parameters first, before building the instruction message. This means we need to loop over the writes twice.

The Iterator trait isn't great for that. We could demand Iterator + Clone, but some iterator implementations might clone their data.

There are ways around it, but all of them require allocations. So I figured it's ok to limit the input to a slice and make the caller responsible for the allocation.

But I'm always open to suggestions to improve this :)

One option would be to document clearly that the iterator will be cloned, so that you should use a cheaply clonable iterator by preference.

[de-vri-es]

Released with v0.9.0 🚀