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

Provide very bare-bones minimal success/error codes for CAN API. #237

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

Conversation

aentinger
Copy link
Contributor

This is related to arduino/ArduinoCore-mbed#956 as well as to arduino/ArduinoCore-mbed#924 .

The underlying problem is that different HALs provide different errors and different error codes when it comes to any peripheral usage (though we concern ourselves with CAN in this PR).

I propose to add at least those two very generic error codes at this very moment with the aim to expand error codes (i.e. CAN_TX_FIFO_FULL) further in the future.

@aentinger aentinger self-assigned this Sep 20, 2024
@aentinger aentinger force-pushed the can/minimal-error-codes branch from 9cd581f to 6c411fe Compare September 20, 2024 07:59
@aentinger aentinger marked this pull request as ready for review September 20, 2024 07:59
@aentinger
Copy link
Contributor Author

Thoughts @facchinm ?

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.53%. Comparing base (4a02bfc) to head (6c411fe).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   95.53%   95.53%           
=======================================
  Files          16       16           
  Lines        1075     1075           
=======================================
  Hits         1027     1027           
  Misses         48       48           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@facchinm
Copy link
Member

Definitely agree on the concept. On the numbering, it really depends on the expected pattern to consume these values. Eg:
while (!can.write()) could mean "keep trying until the error disappears", so CAN_ERROR_GENERIC should be 0 (which is also consistent with the Print.write API (returns the number of bytes actually written, so 0 bytes in case of error).

Copy link
Member

@facchinm facchinm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment in thread

@aentinger
Copy link
Contributor Author

Ciao @facchinm ☕ 👋

That's allright with me. It is a little bit at odds with the current error definition but I'd say we have a chance right now to create a clean API. What do you think (concerning possible API breakage)?

@facchinm
Copy link
Member

What about adopting Print strategy and add setWriteError/getWriteError while returning 0 on failure?

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants