Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unhandled exception in pdo read #306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NicoPowers
Copy link

This fixes the unhandled exception SdoCommunicationError that is raised in the SdoClient when trying to read PDO parameters

This fixes the unhandled exception SdoCommunicationError that is raised in the SdoClient when trying to read PDO parameters
@acolomb
Copy link
Collaborator

acolomb commented Apr 3, 2022

Could you explain a little more please? What kind of error are you seeing? What's the cause? What kind of CANopen node are you talking to?

@NicoPowers
Copy link
Author

Could you explain a little more please? What kind of error are you seeing? What's the cause? What kind of CANopen node are you talking to?

Yes sorry, I am interfacing with a chinese servo motor and it doesn't receive an SDO response for event timers or SYNC starts, and the function that looks for these parameters in the SdoClient will raise an SdoCommunicationError; which is currently not handled by the rpdo and tpdo read functions.

@acolomb
Copy link
Collaborator

acolomb commented Apr 4, 2022

What does the device send then? An SDO abort code? Nothing? Just silencing arbitrary error classes will hide problems that other people may need to notice, drill into, and fix properly at the source. That's why I'm asking for more details, so we can make a targeted fix or make it opt-in for devices that don't adhere to the spec completely, instead of making error diagnosis harder for everyone.

If you search for the name Curtis in the source code, there are already some quirks implemented for nonconforming controllers, but in a way that conforming ones still get the correct treatment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants