search

arduino / ArduinoCore-API

Hardware independent layer of the Arduino cores defining the official API
https://www.arduino.cc/reference/en/
GNU Lesser General Public License v2.1
216 stars 120 forks source link

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

Open aentinger opened 2 months ago

aentinger commented 2 months ago

This is related to https://github.com/arduino/ArduinoCore-mbed/pull/956 as well as to https://github.com/arduino/ArduinoCore-mbed/issues/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 commented 2 months ago

Thoughts @facchinm ?

codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

facchinm commented 2 months ago

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).

aentinger commented 2 months ago

Ciao @facchinm :coffee: :wave:

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 commented 2 months ago

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