atsign-foundation / at_c

Experimental cross-platform C implementation of the atSDK for SOC & embedded devices
BSD 3-Clause "New" or "Revised" License
2 stars 3 forks source link

at_c: handle our own error/warning codes #122

Open realvarx opened 2 months ago

realvarx commented 2 months ago

MbedTLS uses 16 bits to represent errors, they do this to support more devices, with the following format:

 * 1 bit  - Unused (sign bit)
 * 3 bits - High level module ID
 * 5 bits - Module-dependent error code
 * 7 bits - Low level module errors

(U HHH MMMMM LLLLLLL)

We want to be sure that return values representing our error codes dont collide with MbedTLS ones. For instance:

Should we try to fit our error codes in the 16 bits space (for the same reasons MbedTLS does ('we try to keep all error codes within the negative space of 16 bit signed integers to support all 16-bit and 32-bit platforms (-0x0001 - -0x7FFF)') or should be use more bits to be sure that we don't have future collisions?

(we cant use positive space because that represents how much information was tx by some methods)

Other alternatives might consider the use of an extra output parameter in the methods that need it (although this would divide our error system into the integer return value of the method and the output parameter (2 error outputs in some methods), which could become more tedious to handle.

Links: https://mbed-tls.readthedocs.io/en/latest/kb/development/how-are-error-codes-defined/ https://github.com/Mbed-TLS/mbedtls/blob/development/include/mbedtls/error.h https://gist.github.com/erikcorry/b25bdcacf3e0086f8a2afb688420678e

realvarx commented 2 months ago

Yesterday, the development team held a meeting where the topic of this ticket was discussed. Both @cpswan and @XavierChanth expressed that it was a good idea to try to express our error codes through the return variable (and not through an extra output parameter).

Particularly, @cpswan proposed to use another 16 bits in addition to those used by MbedTLS (reaching a total of 32 bits), with which we could comfortably express our error codes without fear of collisions with those of MbedTLS. Also, we could use one of the bits as a flag to inform the user if there is also an error with MbedTLS (in addition to those corresponding to our functions).

This last one seems to be the solution that several developers have found most reasonable.

XavierChanth commented 2 months ago

The only thing stopping us is that we won't be able to support 16bit processors, but the main targets for microcontrollers right now are esp32 and secondarily stm32, both of which are 32bit.

XavierChanth commented 2 months ago

Both @cpswan and @XavierChanth expressed that it was a good idea to try to express our error codes through the return variable (and not through an extra output parameter).

I also mentioned that we should try to avoid returning non-error code things from the return value, such as how mbedtls returns sent / received length as a positive integer. that would free up the positive integer space to use with our own return codes, without having to expand the return size beyond 16bits. Although to be honest, can we find a 16bit processor that is powerful enough to run our hardware? Should we even care to try and support it? On that I'm not sure.

cpswan commented 2 months ago

I wouldn't worry too much about 16bit MCUs, as they're generally going to be too resource starved to run even a lightweight version of our stuff.

I know that there's still 8bit and 16bit stuff out there, but in the conversations we've had about systems there's usually something newer and more capable dealing with Internet connectivity.

ksanty commented 2 months ago

Hi @realvarx who is working on this and how many SP's?

realvarx commented 1 month ago

Moving this to Backlog, as it serves more as a reminder