Skip to content
New issue

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

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

Already on GitHub? # to your account

CAN.write() return-code does not comply to API spec #924

Closed
samuelsadok opened this issue Jul 19, 2024 · 2 comments · Fixed by #956
Closed

CAN.write() return-code does not comply to API spec #924

samuelsadok opened this issue Jul 19, 2024 · 2 comments · Fixed by #956
Assignees
Labels
bug Something isn't working Portenta

Comments

@samuelsadok
Copy link

CAN.write() in this core returns 0 for errors but should return a negative error code.

Details

The docstring for the Arduino CAN API states that HardwareCAN::write() returns:

1 if the message was enqueued, an implementation defined error code < 0 if there was an error

This core implements HardwareCAN::write() by calling mbed::CAN::write() and directly returning the result (code, header).

However mbed::CAN::write() has a different spec for returning errors:

0 if write failed, 1 if write was successful

Discussion

This was brought up by @bthacher-crabi in odriverobotics/ODriveArduino#4 and I'd be interested what the maintainers of this repo think. Technically the meaning of return 0 is unspecified in the API, so it's unclear what client code should do with it. It's conceivable that other cores use 0 to mean "success" (which is a fairly common convention). Should we add a preprocessor special case for the mbed core specifically?

Separately, going forward it would be cool if either the code or the API specs are changed so that they are consistent.

@samuelsadok
Copy link
Author

Any comment on this? (perhaps @facchinm?)

@aentinger aentinger added bug Something isn't working Portenta labels Sep 16, 2024
@aentinger aentinger self-assigned this Sep 16, 2024
@aentinger
Copy link
Contributor

Hi @samuelsadok ☕ 👋 Thank you for reporting this, you are indeed correct. I'll prepare a PR with a fix.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working Portenta
Projects
None yet
2 participants