refactor: simplify coe sync manager types #330
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
theol0403 force-pushed the sync-manager-types branch from dce898b to b17a094 Compare 
Collaborator
| 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 :) |
Author
| @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 :) |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
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
0x1C00were 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_typewhich was manually discovered tosync_manager.usage_typeand deleting unused code results in some simplification (and alignment with EEPROM pdo discovery), with the added effect of allowing my device to boot.