Skip to content

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

Closed
@samuelsadok

Description

@samuelsadok

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.

Activity

samuelsadok

samuelsadok commented on Sep 16, 2024

@samuelsadok
Author

Any comment on this? (perhaps @facchinm?)

self-assigned this
on Sep 16, 2024
aentinger

aentinger commented on Sep 16, 2024

@aentinger
Contributor

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

added a commit that references this issue on Sep 20, 2024
382f4f4
added a commit that references this issue on Sep 26, 2024
931f0bb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

PortentabugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Participants

    @aentinger@samuelsadok

    Issue actions

      CAN.write() return-code does not comply to API spec · Issue #924 · arduino/ArduinoCore-mbed