I2C abort reason #1370
I2C abort reason #1370
Conversation
peterharperuk
force-pushed
the
add_last_abort_reason
branch
from
30f37a6
to
37a61a7
Compare
|
Fixed build issue |
|
assigning to myself; i think i'd rather return the abort reason in the top half of a 64 bit rc (and do some API massaging) |
I don't think you liked this change? I could add a variant of the functions to return int64_t if that sounds like what you wanted? |
|
yeah, i guess returning a struct with two elements in a replacement APIs and picking the result out for the existing APIs is good |
peterharperuk
force-pushed
the
add_last_abort_reason
branch
from
37a61a7
to
60f5901
Compare
Currently we're returning PICO_ERROR_GENERIC for an i2c abort such as "No ack". Add a new PICO_ERROR_ABORT and use this if PICO_I2C_RETURN_ABORT_REASON is true. The abort reason comes from tx_abrt_source (I2C_IC_TX_ABRT_SOURCE_ABRT_*_BITS) so take the zero count from PICO_ERROR_ABORT, so the reason can be determined from the return code. Fixes raspberrypi#1049
peterharperuk
force-pushed
the
add_last_abort_reason
branch
from
60f5901
to
4ac16d6
Compare
|
I've had another go at this. I decided I couldn't face adding new overloads to an already complicated API, just to get a not-often used reason for an abort. Instead I've added a new PICO_ERROR_ABORT error code. This is only used if PICO_I2C_RETURN_ABORT_REASON=1 in which case the abort reason is taken from this. So you can obtain the reason with a bit of maths. |
|
interesting... i think perhaps i'd prefer to make something more generic (and not overlaying existing/future return codes)... e.g. assign codes can then add a |
|
Sounds good, will have a go at that. The one issue is that the docs specifies that PICO_ERROR_GENERIC is returned if there's no ack (abort basically), that's the reason I haven't enabled this by default as it's basically an API breakage? |
|
yes; we have hit this elsewhere - hard to change from -1, though we want to move people towards |
| @@ -42,7 +42,8 @@ enum pico_error_codes { | |||
| PICO_ERROR_UNSUPPORTED_MODIFICATION = -18, ///< Write is impossible based on previous writes; e.g. attempted to clear an OTP bit | |||
| PICO_ERROR_LOCK_REQUIRED = -19, ///< A required lock is not owned | |||
| PICO_ERROR_VERSION_MISMATCH = -20, ///< A version mismatch occurred (e.g. trying to run PIO version 1 code on RP2040) | |||
| PICO_ERROR_RESOURCE_IN_USE = -21 ///< The call could not proceed because requires resourcesw were unavailable | |||
| PICO_ERROR_RESOURCE_IN_USE = -21, ///< The call could not proceed because requires resourcesw were unavailable | |||
There was a problem hiding this comment.
nitpick: "requires resourcesw" -> "required resources"
| @@ -130,6 +130,15 @@ void i2c_set_slave_mode(i2c_inst_t *i2c, bool slave, uint8_t addr) { | |||
| i2c->hw->enable = 1; | |||
| } | |||
|
|
|||
| // PICO_CONFIG: PICO_I2C_RETURN_ABORT_REASON, change i2c functions to return the abort reason via a return code less than or equal to PICO_ERROR_ABORT, type=bool, default=0, group=harware_i2c | |||
Fixes #1049