Simplify CoE sync manager types

[theol0403]

Re-opened from #324, my bad for the noise.

I was debugging a new actuator and trying to understand why it wasn't booting.

Turns out the sm types in 0x1C00 were incorrectly reported by CoE, resulting in RxPDO being treated as TxPDO. This is not ethercrab's fault.

However, while investigating the code, I noticed what appears to be code that not only was redundant, but was succumbing to this bug.

During CoE pdo discovery, we already know all the SM types from sync_managers, as we zip it anyway ever since #314.

Switching from sm_type which was manually discovered to sync_manager.usage_type and deleting unused code results in some simplification (and alignment with EEPROM pdo discovery), with the added effect of allowing my device to boot.

[jamwaffles]

No worries for the noise! I appreciate cleanups and optimisations but I'm a bit swamped at the moment so it'll be a while before I can review even this short PR. Thanks for being patient :)

[theol0403]

@jamwaffles no worries at all! It's no rush on my end, I'm totally good maintaining a fork. Just contributing improvements as I go along, with the ultimate goal of deleting my fork one day :)

[jamwaffles]

I've had a read through the code and reasoning (thanks heaps for typing out your thought process!) and this is a tricky situation as this fix might make your SubDevice work, but regress someone else's. The EEPROM/CoE config stuff has always been a pain and the real solution is to use ESI files instead, but EtherCrab won't support those for a while yet...

IIRC the original behaviour in EtherCrab was copied from SOEM, however leaves out a bunch of SubDevice bug prevention code which would handle cases like yours. Do you think it's worth adding that logic to EtherCrab when it reads CoE SM types? I'm open to the simpler code but it's difficult to test this stuff for regressions without having a huge hardware test suite (IMO anyway). Let me know what you think :)

P.s. I rebased your PR to freshen up the CI, hope you don't mind.

[theol0403]

Hey! The purpose of this PR wasn't explicitly to fix my device, but generally to reflect what I've found with dealing with CoE. In general, it seems like CoE can't be trusted -- some devices don't support it, and some devices (like mine) are just plain wrong. CoE seems like more of a secondary source that you can use to read system parameters in a dictionary, but not necessarily the source of truth when setting up sync managers etc. This is especially true since you need the sync managers already initialized to enable CoE communications.

In ethercrab, the sync managers and their types are provided by EEPROM and iirc mandatory for functionality (ie if EEPROM sync manager types are wrong, nothing works anyway because you can't even init mailboxes). But then, ethercrab currently ALSO asks CoE for SM types and uses that only in some cases (eg to index during PDO discovery). This seems like a hack that uses non-official channels to obtain information we should already have (and the singular nature of the code I deleted kinda reflects this).

Tricky if true that SOEM follows existing behavior. I only have experience with the ~3 actuator manufacturers I've worked with. Hard to predict other regressions. But, worth noting that iirc using CoE for SM init is against the spec anyway.

Regarding patches, feel free to take over this PR; not actively working on this project atm.

[jamwaffles]

Ok, that explanation makes a lot of sense, thanks. I'll fix up this PR when I can recapture the tests with my dev bench (they're broken because the wire traffic is different now) and get it merged :).

I made the changes myself lol