Flash device emulation w/ MCI driver for native #4345
Conversation
x3ro
added
the
Type: new feature
| E_FS_OUT_OF_RANGE = 3, //!< The operation was out of range | ||
| E_FS_FILE_ERROR = 4, //!< Error operating on the file simulating the flash device | ||
| E_FS_NOT_INITIALIZED = 5 //!< Flash simulation layer was not initialized prior to operation | ||
| } flash_sim_error_t; |
There was a problem hiding this comment.
Why does flash_sim needs it's own error class?
There was a problem hiding this comment.
I wanted to have meaningful errors tailored to the module. What would you suggest I use?
There was a problem hiding this comment.
As far as I see everyone of these can be expressed through an errno.
E_FS_SUCCESS = 0
E_FS_INVALID_WRITE = -EIO /* Input/output error */
E_FS_INVALID_PARAMS = -EINVAL /* Invalid argument */
E_FS_OUT_OF_RANGE = -EFAULT /* Bad address */
E_FS_FILE_ERROR = /* multiple errnos related to files, depending on what you want, see errno(3) */
E_FS_NOT_INITIALIZED = -ENODEV /* No such device */
Making up new error classes for modules is not very meaningful or helpful IMHO, since:
- most users don't check them anyways (only IF there was an error)
- it makes passing errors up more complicated, since modules passing up the error have to first check what it was and then use their respective error code
- it decouples documentation of the error case from the actual place where it happens. Your code already just states
E_SUCCESS or another ::flash_sim_error_t. That makes it really hard for testers to say if a function can error onE_FS_INVALID_WRITE,E_FS_OUT_OF_RANGE, etc.
There was a problem hiding this comment.
Makes sense. Thanks for the input :)
There was a problem hiding this comment.
Do you think this is also the case for more complex errors? When interacting with a flash device, for example, I might encounter ECC errors or a page I'm trying to read from may not have been written too and thus cannot be read. While I could represent both of those errors with EIO, being able to distinguish such error cases is necessary since they need to be handled differently. What would be a good solution in such a case?
There was a problem hiding this comment.
In most cases there is some kind of error code that can loosely describe the behavior. For the ECC error EIO seems like a good idea, or EBADF. For the other thing EBADFD might be a good code. You are the only one using the error codes in that scope and they can be generalized more, when you hand them upstream. Get creative ;-)
x3ro
force-pushed
the
ma/flash-emulation
branch
from
69b1472 to
fde29f1
Compare
x3ro
force-pushed
the
ma/flash-emulation
branch
2 times, most recently
from
4c28d4c to
4ed4677
Compare
|
Does anyone volunteer to take over this PR? |
[flashsim] Fix broken unit tests [flashsim] Add documentation [flashsim] Properly namespace error enum [flashsim] Fix header not renamed [fs] replace custom error codes with errnos
x3ro
force-pushed
the
ma/flash-emulation
branch
from
4ed4677 to
3b5d7c2
Compare
|
Won't be able to review until the next release, sorry Lucas! |
|
Who has time and knowledge to review this PR? I will remove the Milestone label for now. |
|
So, what do we do with this PR? |
|
needs rebase and adoption, until then I'll remove milestone |
|
No progress for over a year. Closing as memo for now. |
miri64
added
the
State: archived
Supersedes #4033
Depends on #4220
I propose the following "flash simulation layer" for developing/testing RIOT applications, intended to access flash-based storage, on native. It emulates the following characteristics of flash storage:
Also included is an implementation, based on the flash simulation layer, for the existing MCI storage interface (implemented e.g. for the MSBA2 board).