Add first-class PDO discovery and manipulation #317
Conversation
|
Reduced stack size by using Edit: will add a trivial alloc flag, let me know if this is acceptable. |
|
Hey, thanks for digging into this a bit. I started some work on this a while ago and have just opened #318 in a very rough state to keep an eye on it. I think I'd prefer to go in my own direction for this kind of functionality, but I'd be very interested to hear your thoughts on that PR when it's in a better state. |
|
I believe our two approaches are mostly orthogonal and likely complimentary. Currently, to access the pdi, one must do the following: let mut i = subdevice.inputs_raw_mut();
// byte slice indices require knowledge of ethercat internals and device-specific config
let status = u16::from_le_bytes(i[4..=5].try_into().unwrap()); This code must be hardcoded per device. My usecase, and this PR, is to be able to do the following: let mut io = subdevice.io_raw_mut();
// read status word regardless of config
let mut inputs = io.inputs();
let status: u16 = inputs.pdo_read(0x6041, 0).unwrap();
// write control word regardless of config
let mut outputs = io.outputs();
if let Err(e) = outputs.pdo_write(0x6040, 0, 6u8) {
println!("PDO isn't mapped in PDI image: {e:?}");
// fallback to sdo
subdevice.sdo_write(0x6060, 0, 6u8);
}Or automatically print all input PDOs for debugging:let inputs = io.inputs();
for ((index, sub_index), (_, len)) in subdevice.io_segments().tx_pdos.iter() {
#[allow(dead_code)]
#[derive(Debug)]
enum Fmt {
I8(i8),
I16(i16),
I32(i32),
I64(i64),
}
let fmt = match len {
1 => Fmt::I8(inputs.pdo_read(*index, *sub_index).unwrap()),
2 => Fmt::I16(inputs.pdo_read(*index, *sub_index).unwrap()),
4 => Fmt::I32(inputs.pdo_read(*index, *sub_index).unwrap()),
8 => Fmt::I64(inputs.pdo_read(*index, *sub_index).unwrap()),
_ => panic!("Unsupported bit length"),
};
log::info!(
"SubDevice {:#06x} index: {:#04x}, subindex: {:#04x}, value: {:?}",
subdevice.configured_address(),
index,
sub_index,
fmt
);
}This is done completely transparently to the user, with no extra code (or config). Other than the mapping stack storage, this is also done completely for free, piggybacking on the sm discovery we already do. In my usecase, I have multiple servos from different manufacturers connected to the same bus. They all have different FMMU configs, with io byte indexes in different places. However, they are guaranteed to map, at some location, the crucial PDOs for DS402 operation. Most importantly, their rx/tx pdo mappings are read-only and cannot be configured. From my understanding, your upcoming work is to allow users to explicitly configure their sm configs. However, this will not help with my usecase as writing config to the device will panic. Instead, this PR allows users to write the same generic code without needing any device-specific hardcoded logic (or ESI), using greatly improved ergonomics. This PR also DRYs the pdi.rs a little bit. While I think providing the ability for manual config in #318 is very useful, it solves a different usecase, and the mechanisms for named PDI access can be complimentary. Feel free to take as many or as little as the ideas from this as you want. I'll maintain my fork for my usecase until/if equivalent functionality is available in main. |
d9cd33e to
9569a02
Compare
Thanks again for this awesome crate!
When learning ethercrab (and ethercat), it was difficult and unclear to understand exactly how PDOs are managed, how to access them, and specifically the byte mappings to the PDI input/output slices. Not only this, but the ergonomics of dealing with raw byte slices are poor and brittle.
When digging into the crate, I realized that we already have all the information we need to provide a richer PDO interface, through the sm bit length discovery we conduct during init.
This PR aims to provide a clean and easy way to interact with PDOs. It does this by:
pdi.rsby delegatingPdiIoRawWriteGuardtoPdiWriteGuard(ditto for read) and simplifying ownershipfn pdo_rawonPdiWriteGuard(and read) which produces a slice based on PDO mappingsI believe this is the first step to richer object dictionary support and flexible device support. My usecase here is connecting to varous Ds402 and having them work out of the box (provided their default configurations provide the required PDO mappings) without needing to know their PDI byte mappings during compile time.
I'd love to hear your thoughts on this design.
The largest challenge I am running into: stack space. Currently,
heapless::FnvIndexMap<(u16, u8), Range<usize>, 32>;is904bytes on a macbook pro. This, multiplied by two, crashes the stack if you have many devices enabled.Range<u16>reduces to 456 bytes, as our PDI will never be that huge, but then the slice accessors become more messy (acceptable)LinearMapreduces to 264 bytes, which is better, but has a runtime costImplementing both these approaches still overflow the stack in some instances.
Perhaps the correct solution is to move PDI mapping config into static storage along with the PDI itself, though I don't really know how to do this cleanly. Alternatively, we could add an
allocfeature flag for use in any future object-dictionary related features. Any suggestions?Todo:
pdo_read<T>andpdo_write<T>non-raw methods toPdiWriteGuardio_raw_muttoio_mut, as it will support raw and non-raw access? Same with the other raw methods.as_slice()? That way we can haveinputs.as_sliceorinputs.pdo_read.pdo_rawpublic access, as a byte slice implements WireRead/Write so can be used withpdo_read/write?AddNot needed with access toall_pdoswhich returns a map of all mapped PDOs and their corresponding byte slices?io_segments.self.lock.get_mut()[self.range.clone()]on the read-only guards too?