ethercrab-rs / ethercrab

EtherCAT master written in pure Rust
258 stars 25 forks source link

Consider adding API to manually provide metadata about PDOs? #228

Open david-boles opened 4 months ago

david-boles commented 4 months ago

Hello! I'm attempting to use a Kistler 5074B with ethercrab but configure_pdos_coe fails because the PDO mapping SDOs seem to not be readable. E.g. manually running in pre-op:

slave.sdo_read::<u8>(0x1600, 0).await.unwrap();
slave.sdo_read::<u32>(0x1600, 1).await.unwrap();
slave.sdo_read::<u8>(0x1A00, 0).await.unwrap();
slave.sdo_read::<u32>(0x1A00, 1).await.unwrap();

All fail:

Mailbox error for slave 0x1000 (supports complete access: false): 0x06040043: General parameter incompatibility reason

Even though those definitely do correspond to (fixed) PDOs in the ESI:

<RxPdo Sm="2" Mandatory="false" Fixed="true">
  <Index>#x1600</Index>
  <Name>Control Ch1</Name>
  <Entry>
    <Index>#x2000</Index>
    <SubIndex>1</SubIndex>
    <BitLen>8</BitLen>
    <Name>Control Ch1 (0)</Name>
    <DataType>BYTE</DataType>
  </Entry>
</RxPdo>
<TxPdo Sm="3" Mandatory="false" Fixed="true">
  <Index>#x1A00</Index>
  <Name>Status Ch1</Name>
  <Entry>
    <Index>#x3000</Index>
    <SubIndex>1</SubIndex>
    <BitLen>8</BitLen>
    <Name>Status Ch1</Name>
    <DataType>BYTE</DataType>
  </Entry>
</TxPdo>

Attempting to set up from eeprom also fails, though I want to be able to configure the PDO assignments and so didn't look into why all that closely.

Hopefully I'm not just doing something dumb; EtherCAT's new to me :smile:

I have had success manually configuring the PDO assignments, SMs, and FMMUs, however there's no way for me to "inform" the group that I have done so, or, for example, use the PDI offset to play nicely with automatic setup of the other slaves.

If this is something you want to support, and you have a suggestion for what would be a sensible, acceptable API design, I might have time to implement it myself. Or, if I'm missing a better solution, I'd love to know.

Thanks for any input, and this lovely crate!

For reference, here's what I've gotten working so far: ``` rust //! Kistler 5074b Charge Amplifier //! //! cargo build --example kistler_5074b && sudo setcap cap_net_raw=eip target/debug/examples/kistler_5074b && RUST_LOG=debug RUST_BACKTRACE=1 target/debug/examples/kistler_5074b enx40ae306d11a8 use env_logger::Env; use ethercrab::{ slave_group::{NoDc, PreOpPdi}, std::{ethercat_now, tx_rx_task}, Client, ClientConfig, Command, PduStorage, RegisterAddress, SlaveGroup, Timeouts }; use futures_lite::StreamExt; use std::{ops::Deref, sync::{atomic::{AtomicBool, Ordering}, Arc}, time::Duration}; use ethercrab_wire::EtherCrabWireRead; /// Maximum number of slaves that can be stored. This must be a power of 2 greater than 1. const MAX_SLAVES: usize = 128; /// Maximum PDU data payload size - set this to the max PDI size or higher. const MAX_PDU_DATA: usize = PduStorage::element_size(1100); /// Maximum number of EtherCAT frames that can be in flight at any one time. const MAX_FRAMES: usize = 16; /// Maximum total PDI length. const PDI_LEN: usize = 500; static PDU_STORAGE: PduStorage = PduStorage::new(); fn main() { env_logger::Builder::from_env(Env::default().default_filter_or("info")).init(); let interface = std::env::args() .nth(1) .expect("Provide network interface as first argument."); let (tx, rx, pdu_loop) = PDU_STORAGE.try_split().expect("can only split once"); let client = Arc::new(Client::new( pdu_loop, Timeouts::default(), ClientConfig { dc_static_sync_iterations: 0, ..ClientConfig::default() }, )); smol::block_on(async { smol::spawn(tx_rx_task(&interface, tx, rx).expect("spawn TX/RX task")).detach(); let mut group = client .init_single_group::(ethercat_now) .await .unwrap(); // Configure PDI, disable watchdogs let pdi_length_bytes = { let slave = group.slave(&client, 0).unwrap(); // slave.sdo_read::(0x1600, 0).await.unwrap(); // slave.sdo_read::(0x1600, 1).await.unwrap(); // slave.sdo_read::(0x1A00, 0).await.unwrap(); slave.sdo_read::(0x1A00, 1).await.unwrap(); let mut pdi_length_bytes = 0; // XXX: EtherCrab does master read/inputs first. This order seems more sensible to me. // Master write (outputs) pdi_length_bytes += { const SM_IDX: u8 = 2; const FMMU_IDX: u8 = 0; const PHYSICAL_START_ADDRESS: u16 = 0x1100; // PDO mapping // All PDOs are fixed, nothing to do. // PDO assignments // XXX: You have to play this little song and dance with the CoE array's length. slave.sdo_write::(0x1C12, 0, 0).await.unwrap(); // Control Ch - BYTE // XXX: Even though these are the default PDO assignments, you do have to reassign them for whatever reason :( slave.sdo_write::(0x1C12, 1, 0x1600).await.unwrap(); slave.sdo_write::(0x1C12, 2, 0x1610).await.unwrap(); slave.sdo_write::(0x1C12, 3, 0x1620).await.unwrap(); slave.sdo_write::(0x1C12, 4, 0x1630).await.unwrap(); // Donzo slave.sdo_write::(0x1C12, 0, 4).await.unwrap(); let additional_pdi_bytes: u16 = 4*1; // Configure SM Command::fpwr(slave.configured_address(), RegisterAddress::sync_manager(SM_IDX).into()).send(&client, SyncManagerChannel { physical_start_address: PHYSICAL_START_ADDRESS, length_bytes: additional_pdi_bytes, control: Control { operation_mode: OperationMode::Normal, direction: Direction::MasterWrite, ecat_event_enable: true, dls_user_event_enable: true, // XXX: Doesn't matter since all will be disabled but might as well match the ESI XML regardless *shrug* watchdog_enable: true, }, status: Status::default(), enable: Enable { enable: true, ..Enable::default() }, }) .await.unwrap(); // Configure FMMU Command::fpwr(slave.configured_address(), RegisterAddress::fmmu(FMMU_IDX).into()).send(&client, Fmmu { logical_start_address: pdi_length_bytes.into(), length_bytes: additional_pdi_bytes, // Mapping into PDI is byte-aligned until/if we support bit-oriented slaves logical_start_bit: 0, // Always byte-aligned logical_end_bit: 7, physical_start_address: PHYSICAL_START_ADDRESS, physical_start_bit: 0, read_enable: false, write_enable: true, enable: true, }).await.unwrap(); additional_pdi_bytes }; // Master read (inputs) pdi_length_bytes += { const SM_IDX: u8 = 3; const FMMU_IDX: u8 = 1; const PHYSICAL_START_ADDRESS: u16 = 0x1400; // PDO mapping // All PDOs are fixed, nothing to do. // PDO assignments // XXX: You have to play this little song and dance with the CoE array's length. slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 0, 0).await.unwrap(); // Status ChX - BYTE slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 1, 0x1A00).await.unwrap(); slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 2, 0x1A10).await.unwrap(); slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 3, 0x1A20).await.unwrap(); slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 4, 0x1A30).await.unwrap(); // Scaled Value ChX - REAL slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 5, 0x1A01).await.unwrap(); slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 6, 0x1A11).await.unwrap(); slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 7, 0x1A21).await.unwrap(); slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 8, 0x1A31).await.unwrap(); // Donzo slave.sdo_write::(SM_BASE_ADDRESS + u16::from(SM_IDX), 0, 8).await.unwrap(); let additional_pdi_bytes: u16 = (4*1) + (4*4); // Configure SM Command::fpwr(slave.configured_address(), RegisterAddress::sync_manager(SM_IDX).into()).send(&client, SyncManagerChannel { physical_start_address: PHYSICAL_START_ADDRESS, length_bytes: additional_pdi_bytes, control: Control { operation_mode: OperationMode::Normal, direction: Direction::MasterRead, ecat_event_enable: true, dls_user_event_enable: true, watchdog_enable: false, }, status: Status::default(), enable: Enable { enable: true, ..Enable::default() }, }) .await.unwrap(); // Configure FMMU Command::fpwr(slave.configured_address(), RegisterAddress::fmmu(FMMU_IDX).into()).send(&client, Fmmu { logical_start_address: pdi_length_bytes.into(), length_bytes: additional_pdi_bytes, // Mapping into PDI is byte-aligned until/if we support bit-oriented slaves logical_start_bit: 0, // Always byte-aligned logical_end_bit: 7, physical_start_address: PHYSICAL_START_ADDRESS, physical_start_bit: 0, read_enable: true, write_enable: false, enable: true, }).await.unwrap(); additional_pdi_bytes }; // Disable PSI and all sync manager watchdogs via timer scaler // XXX: Saw a note that this might not be supported for all slaves Command::fpwr(slave.configured_address(), 0x0410u16).send(&client, 0u16).await.unwrap(); Command::fpwr(slave.configured_address(), 0x0420u16).send(&client, 0u16).await.unwrap(); pdi_length_bytes }; // XXX: PDI has now been configured manually, though ethercrab's metadata about the PDI has not so we'll have to interact with it ourselves. let group = unsafe { std::mem::transmute::<_, SlaveGroup>(group)}; group.into_op(&client).await.unwrap(); fn print_pdi(pdi: &[u8]) { println!(); for i in 0..4 { println!("Channel {i} control: {:08b}", pdi[i]); } for i in 0..4 { println!("Channel {i} status: {:08b}", pdi[4+i]); } for i in 0..4 { let start = 8 + (4*i); let end = start + 4; println!("Channel {i} value (pC): {:12.3}", f32::unpack_from_slice(pdi[start..end].try_into().unwrap()).unwrap()); } } async fn tx_rx<'client>(client: &'client Client<'client>, pdi: &mut [u8]) { // XXX: Function signature lifetimes should be changed such that allocating a new vec isn't necessary. let received = Command::lrw(0).with_wkc(3).send_receive_slice(&client, &*pdi).await.unwrap().deref().to_owned(); pdi.copy_from_slice(&received); print_pdi(&pdi); } let mut pdi = vec![0u8; pdi_length_bytes.into()]; // Zero the charge amp. println!("\nSTARTING"); pdi[..4].fill(0); tx_rx(&client, &mut pdi).await; assert!(pdi[4..8].iter().all(|status| status & 1 == 0)); println!("\nWAITING FOR ZERO TO COMPLETE"); pdi[..4].fill(1); while pdi[4..8].iter().any(|status| status & 1 == 0) { tx_rx(&client, &mut pdi).await; } println!("\nBEGINNING OPERATION"); let mut cycle_time = smol::Timer::interval(Duration::from_millis(1000)); let shutdown = Arc::new(AtomicBool::new(false)); signal_hook::flag::register(signal_hook::consts::SIGINT, Arc::clone(&shutdown)) .expect("Register hook"); loop { // Graceful shutdown on Ctrl + C if shutdown.load(Ordering::Relaxed) { log::info!("Shutting down..."); break; } tx_rx(&client, &mut pdi).await; cycle_time.next().await; } }); log::info!("Done."); } // =============== COPIED FROM ETHERCRAB ================= /// // ETG1000.6 Table 67 – CoE Communication Area, the address of the first sync manager. pub const SM_BASE_ADDRESS: u16 = 0x1C10; #[derive(Default, Copy, Clone, Debug, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bits = 2)] #[repr(u8)] pub enum OperationMode { #[default] Normal = 0x00, Mailbox = 0x02, } #[derive(Default, Copy, Clone, Debug, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bits = 2)] #[repr(u8)] pub enum Direction { #[default] MasterRead = 0x00, MasterWrite = 0x01, } #[derive(Default, Copy, Clone, Debug, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bytes = 1)] pub struct Control { #[wire(bits = 2)] pub operation_mode: OperationMode, #[wire(bits = 2)] pub direction: Direction, #[wire(bits = 1)] pub ecat_event_enable: bool, #[wire(bits = 1)] pub dls_user_event_enable: bool, #[wire(bits = 1, post_skip = 1)] pub watchdog_enable: bool, // reserved1: bool } #[derive(Default, Copy, Clone, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bytes = 8)] pub struct SyncManagerChannel { #[wire(bytes = 2)] pub physical_start_address: u16, #[wire(bytes = 2)] pub length_bytes: u16, #[wire(bytes = 1)] pub control: Control, #[wire(bytes = 1)] pub status: Status, #[wire(bytes = 2)] pub enable: Enable, } /// Buffer state. /// /// Somewhat described in ETG1000.4 Figure 32 – SyncM mailbox interaction. /// /// In cyclic mode the buffers need to be tripled. It's unclear why from the spec but that's what it /// says. #[derive(Default, Copy, Clone, Debug, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bits = 2)] #[repr(u8)] pub enum BufferState { /// First buffer. #[default] First = 0x00, /// Second buffer. Second = 0x01, /// Third buffer. Third = 0x02, /// Next buffer. Next = 0x03, } #[derive(Default, Copy, Clone, Debug, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bytes = 1)] pub struct Status { #[wire(bits = 1)] pub has_write_event: bool, #[wire(bits = 1, post_skip = 1)] pub has_read_event: bool, // reserved1: bool #[wire(bits = 1)] pub mailbox_full: bool, #[wire(bits = 2)] pub buffer_state: BufferState, #[wire(bits = 1)] pub read_buffer_open: bool, #[wire(bits = 1)] pub write_buffer_open: bool, } /// ETG1000.4 Table 56 – Fieldbus memory management unit (FMMU) entity. #[derive(Default, Copy, Clone, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bytes = 16)] pub struct Fmmu { /// This parameter shall contain the start address in octets in the logical memory area of the /// memory translation. #[wire(bytes = 4)] pub logical_start_address: u32, #[wire(bytes = 2)] pub length_bytes: u16, #[wire(bits = 3, post_skip = 5)] pub logical_start_bit: u8, #[wire(bits = 3, post_skip = 5)] pub logical_end_bit: u8, #[wire(bytes = 2)] pub physical_start_address: u16, #[wire(bits = 3, post_skip = 5)] pub physical_start_bit: u8, #[wire(bits = 1)] pub read_enable: bool, #[wire(bits = 1, post_skip = 6)] pub write_enable: bool, // Lots of spare bytes after this one! #[wire(bits = 1, post_skip = 31)] pub enable: bool, } #[derive(Default, Copy, Clone, Debug, PartialEq, Eq, ethercrab_wire::EtherCrabWireReadWrite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[wire(bytes = 2)] pub struct Enable { #[wire(bits = 1)] pub enable: bool, #[wire(bits = 1, post_skip = 4)] pub repeat: bool, // reserved4: u8 /// DC Event 0 with EtherCAT write. /// /// Set to `true` to enable DC 0 events on EtherCAT writes. #[wire(bits = 1)] pub enable_dc_event_bus_write: bool, /// DC Event 0 with local write. /// /// Set to `true` to enable DC 0 events from local writes. #[wire(bits = 1)] pub enable_dc_event_local_write: bool, #[wire(bits = 1)] pub channel_pdi_disabled: bool, #[wire(bits = 1, post_skip = 6)] pub repeat_ack: bool, // reserved6: u8, } ```
jamwaffles commented 4 months ago

Hey, thanks for the detailed report! Would you mind trying some standard EtherCrab code (maybe the discover example, just something that tries some very ordinary initialisation) and post a Wireshark capture of the whole run? If you have Rust logging setup, could you please run your program with RUST_LOG=ethercrab/debug and attach the output here too? I don't have any ideas at the moment but hopefully something stands out in the logs/capture. What version of EtherCrab are you using?

I'd also appreciate the ESI file as I have to log in to the manufacturer's website to download it.

david-boles commented 4 months ago

Absolutely!

4c89cac4741fb2f79c0eb498a205235ffc54e84b (ethercrab-v0.5.0)

I ran both discover and discover with this line added at the end, to cause it to attempt PDI setup:

group.into_op(&maindevice).await.unwrap();

kistler_5074B.zip

david-boles commented 4 months ago

Just to be super explicit, this is the line that's failing (with pdo = 0x1A00): https://github.com/ethercrab-rs/ethercrab/blob/4c89cac4741fb2f79c0eb498a205235ffc54e84b/src/subdevice/configuration.rs#L357-L359

I would guess that my device is just not spec compliant? Unless there's some CoE configuration thing that's making that specific read fail. The tables in ETG1000.6 subsections 5.6.7.4.7 and .8 both indicate that read support is mandatory from what I can tell.

jamwaffles commented 4 months ago

Thanks for the details, much appreciated! I'll have a look and see if I can find anything obvious.

jamwaffles commented 4 months ago

Looking at discover_plus_into_op.pcapng, the last error reported from the mailbox read is 0x0602_0000, or the NotFound variant, however in the adjacent log file discover_plus_into_op.txt the error is 0x06040043: General parameter incompatibility reason. Did any of the code change between the logged run and the Wireshark capture? There's a chance it could be EtherCrab too of course...

I would guess that my device is just not spec compliant?

Possibly but EtherCrab would ideally work around these issues instead of falling flat on its face :D

It might also be worth rerunning at trace level instead of debug and posting that here if you don't mind.

david-boles commented 4 months ago

Looking at discover_plus_into_op.pcapng, the last error reported from the mailbox read is 0x0602_0000, or the NotFound variant, however in the adjacent log file discover_plus_into_op.txt the error is 0x06040043: General parameter incompatibility reason. Did any of the code change between the logged run and the Wireshark capture? There's a chance it could be EtherCrab too of course...

Nope, I still see that too after rerunning.

It might also be worth rerunning at trace level instead of debug and posting that here if you don't mind.

with_trace.zip

jamwaffles commented 4 months ago

As an aside, the error code discrepancy is because EtherCrab hard codes the error on an abort to CoeAbortCode::Incompatible. Not sure why I did that... but it at least explains the different codes between the logs and Wireshark.

Still looking at why the Kistler won't init.

jamwaffles commented 4 months ago

I modified the discover example a bit to get this:

//! Discover devices connected to the network.

use env_logger::Env;
use ethercrab::{
    std::{ethercat_now, tx_rx_task},
    MainDevice, MainDeviceConfig, PduStorage, Timeouts,
};
use std::sync::Arc;

/// Maximum number of SubDevices that can be stored. This must be a power of 2 greater than 1.
const MAX_SUBDEVICES: usize = 128;
/// Maximum PDU data payload size - set this to the max PDI size or higher.
const MAX_PDU_DATA: usize = PduStorage::element_size(1100);
/// Maximum number of EtherCAT frames that can be in flight at any one time.
const MAX_FRAMES: usize = 16;
/// Maximum total PDI length.
const PDI_LEN: usize = 64;

static PDU_STORAGE: PduStorage<MAX_FRAMES, MAX_PDU_DATA> = PduStorage::new();

fn main() {
    env_logger::Builder::from_env(Env::default().default_filter_or("info")).init();

    let interface = std::env::args()
        .nth(1)
        .expect("Provide network interface as first argument.");

    log::info!("Discovering EtherCAT devices on {}...", interface);

    let (tx, rx, pdu_loop) = PDU_STORAGE.try_split().expect("can only split once");

    let maindevice = Arc::new(MainDevice::new(
        pdu_loop,
        Timeouts::default(),
        MainDeviceConfig {
            dc_static_sync_iterations: 0,
            ..MainDeviceConfig::default()
        },
    ));

    smol::block_on(async {
        smol::spawn(tx_rx_task(&interface, tx, rx).expect("spawn TX/RX task")).detach();

        let group = maindevice
            .init_single_group::<MAX_SUBDEVICES, PDI_LEN>(ethercat_now)
            .await
            .expect("Init");

        {
            let sd = group.subdevice(&maindevice, 0).expect("Empty network!");

            log::info!("First SubDevice: {}", sd.name());

            sd.sdo_write(0x1c13, 0, 0u8).await.expect("0x1c13:0 -> 0u8");
            sd.sdo_write(0x1c13, 1, 0x1a00u16)
                .await
                .expect("0x1c13:1 -> 0x1a00");
            sd.sdo_write(0x1c13, 2, 0x1a10u16)
                .await
                .expect("0x1c13:2 -> 0x1a10");
            sd.sdo_write(0x1c13, 3, 0x1a20u16)
                .await
                .expect("0x1c13:3 -> 0x1a20");
            sd.sdo_write(0x1c13, 4, 0x1a30u16)
                .await
                .expect("0x1c13:4 -> 0x1a30");
            sd.sdo_write(0x1c13, 0, 4u8).await.expect("0x1c13:0 -> 4u8");
        }

        log::info!("Discovered {} SubDevices", group.len());

        let mut group = group
            .into_op(&maindevice)
            .await
            .expect("Failed to go into OP");

        for subdevice in group.iter(&maindevice) {
            log::info!(
                "--> SubDevice {:#06x} {} {}",
                subdevice.configured_address(),
                subdevice.name(),
                subdevice.identity()
            );
        }
    });

    log::info!("Done.");
}

Can you see if that does anything? If it doesn't, again please attach a log and Wireshark capture. I think/hope we're just confusing TxPDOs with RxPDOs as the directions in the terminology are reversed to be from the viewpoint of the SubDevice.

david-boles commented 4 months ago

I'll investigate some more on where it failed getting into op but for now:

modified_with_sdo_writes.zip

david-boles commented 4 months ago

Yep, seems like assigning the PDOs didn't make the mapping objects readable, no change. I'll follow up with Kistler themselves to confirm but all their EtherCAT experts are on leave at the moment.

david-boles commented 4 months ago

I think/hope we're just confusing TxPDOs with RxPDOs as the directions in the terminology are reversed to be from the viewpoint of the SubDevice.

Could you explain this a little more? I'm not quite sure how mixing them up could result in this failure. Or even necessarily it would be mixing them up.

I'm also pretty sure you aren't mixing them up? IIRC Ethercrab configures master reads first, and indeed the first attempt to read any PDO mapping length is failing for 0x1A00, which is a TxPDO.

jamwaffles commented 3 months ago

modified_with_sdo_writes.zip

Could you please share the code that produced this trace, so I can match up the packets?

Yep, seems like assigning the PDOs didn't make the mapping objects readable, no change.

I'm still not quite sure why you need to read these PDOs with sdo_read. They're mapped into the PDI and should be read from that data during cyclic operation. I agree it's strange they're not directly readable (or maybe this is normal behaviour - I haven't tried reading them on other SubDevices) but you shouldn't need to.

Could you explain this a little more? I'm not quite sure how mixing them up could result in this failure. Or even necessarily it would be mixing them up.

I was referring to the configuration in your initialisation code as opposed to what happens internally in EtherCrab. I've seen some issues where misconfiguring a SubDevice will prevent it from going into OP with a very cryptic error.

david-boles commented 3 months ago

Could you please share the code that produced this trace, so I can match up the packets?

Oh! Sorry, to clarify, that's the result of running the code you provided here: https://github.com/ethercrab-rs/ethercrab/issues/228#issuecomment-2261636510

(against 4c89cac4741fb2f79c0eb498a205235ffc54e84b).

I'm still not quite sure why you need to read these PDOs with sdo_read.

Ah, sorry for the confusion. I don't. My code, bypassing ethercrab's usual PDI configuration, at the bottom of the original comment, works perfectly fine: https://github.com/ethercrab-rs/ethercrab/issues/228#issue-2439031466

However, ethercrab's setup currently tries to read the PDO mapping object, because it needs to calculate the length of the PDOs that are assigned to the SM, and doesn't have that information provided by the ESI (my impression of what most masters do), or eeprom (in this code path): https://github.com/ethercrab-rs/ethercrab/blob/4c89cac4741fb2f79c0eb498a205235ffc54e84b/src/subdevice/configuration.rs#L357-L359 https://github.com/ethercrab-rs/ethercrab/blob/4c89cac4741fb2f79c0eb498a205235ffc54e84b/src/subdevice/configuration.rs#L401

jamwaffles commented 3 months ago

Oh! Sorry, to clarify, that's the result of running the code you provided here:

No problem :) I should've guessed!

I don't. My code, bypassing ethercrab's usual PDI configuration, at the bottom of the original comment, works perfectly fine:

Right, yes, sorry. Reading your original comment again, it's odd that EEPROM configuration fails as well. Do you remember if it gave any specific errors? Did you modify the EtherCrab internals to force it to configure from EEPROM instead of CoE? I'm starting to think this is all caused by dodgy firmware in the device, but I won't want to deflect blame too quickly.

Would you be open to shipping a device to me so I can debug this issue locally? I appreciate they're expensive and it's a pain, but I'll float the option anyway :)

david-boles commented 3 months ago

it's odd that EEPROM configuration fails as well

I think this may have been because, for whatever reason, my device also needs (at least) the (master write) RxPDO assignments to be written to, even though if you read the assignment subindices they're already in there. That's something else I'm going to follow up with Kistler about...

I'm starting to think this is all caused by dodgy firmware in the device

Oh, I'm absolutely already at that point. Unless there's some other line of text somewhere in the spec that qualifies that "M" (mandatory), it's just, obviously, non-spec compliant. So, my thoughts are around adding an API for working around that, obviously in a way that makes sense for the project as a whole, rather than just this specific device.

Would you be open to shipping a device to me so I can debug this issue locally?

Wow, thanks for the offer! If it comes to it I could probably set up a machine you could ssh into, but I don't think it's worth it... I'll just wait for the Kistler folks to get back and confirm.

jamwaffles commented 3 months ago

I think this may have been because, for whatever reason, my device also needs (at least) the (master write) RxPDO assignments to be written to, even though if you read the assignment subindices they're already in there.

I've seen this behaviour before in other devices. Like they need a complete config written to them even if you're not using most of it, otherwise they fail to init. It's quite frustrating to debug because the error codes you get back are almost beyond cryptic.

For future reference, some/many/most ESI files specify a list of startup commands than tools like TwinCAT will read and show in the UI. I don't think your ESI file has that, but you could try connecting to the Kistler using TwinCAT, seeing if it has init commands, and copying them verbatim into your Rust code.

Oh, I'm absolutely already at that point.

Lol fair, I guess we'll see what Kistler get back with!

So, my thoughts are around adding an API for working around that, obviously in a way that makes sense for the project as a whole, rather than just this specific device.

Yeah that might be necessary, but hopefully not. I'd like to get to the bottom of this issue before designing an API around it.

Wow, thanks for the offer! If it comes to it I could probably set up a machine you could ssh into, but I don't think it's worth it... I'll just wait for the Kistler folks to get back and confirm.

Also a good option! Let's see how far we get with Kistler...

david-boles commented 2 months ago

Hey James, I've been going back and forth with Kistler and unfortunately they've been pretty entirely unhelpful. They seem to be unwilling or unable to connect me with any engineers knowledgeable about more than integrating with an off the shelf PLC.

I've been accumulating a few more EtherCAT devices and I think I'd like to do this properly... write some tooling to parse ESI files, configure the slaves based on a user-given list of PDOs from the ESI, and then pack/unpack values by name from the PDI. Maybe with some macro magic?

If those are features you'd be interested in incorporating into ethercrab and have suggestions on how to do so, I'm open to it!

jamwaffles commented 2 months ago

unfortunately they've been pretty entirely unhelpful.

That's frustrating, but not too surprising in this space :(

If those are features you'd be interested in incorporating into ethercrab and have suggestions on how to do so, I'm open to it!

Absolutely! I think ESI support is even a desired feature in the README 😁. I've just gone back into fulltime work, so I'm afraid I don't have much time to spend on thinking about/workign on this, but from previous thoughts I agree that some macro magic might be a good way to go. I'm not sure what that looks like around e.g. choosing multiple "profiles" from the ESI, but it would mean that some nice easy methods could be generated to configure SubDevices and read/write a semantic representation of their PDI.

I'd also be curious to see what a build.rs-based codegen would look like. This approach would be a little less opaque I think as it would enable at least a little bit more of rust-analyzer's features compared to a proc macro, right?

Either way, if you're happy prototyping something I'd love to see what you come up with. At the very least, it could guide what the feature looks like in EtherCrab.

david-boles commented 2 months ago

Sweet! For starters, I had some problems with https://crates.io/crates/ethercat-esi so I made some progress towards generating ESI deserialization Rust code from the official ETG.2000 schema:

https://github.com/david-boles/ethercat-subdevice-info

I assume you have a much wider variety of ESI files at your disposal than I do... If you'd like to help stress test it you can chuck them in devices and run cargo test :smile: