-
Notifications
You must be signed in to change notification settings - Fork 178
Add FileHandle/poll documentation #745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@kjbracey-arm Thanks for the PR. Whom should I tag for content review? |
ping @geky |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Please create separate pages for FileHandle and Poll because they're separate APIs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Sorry it took so long before I could review it. Thanks for the writing.
Left some suggestions, though it looks good to me.
@@ -0,0 +1,274 @@ | |||
## FileHandle | |||
|
|||
For general information on [files](file.html) and [filing systems](filesystem.html), see the respective documentation. This chapter covers the abstract API, with an emphasis on devices, as they're less straightforward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small suggestion, feel free to tweak/reject:
For general information on [files](file.html) and [filing systems](filesystem.html), see the respective documentation. This chapter covers the abstract API, with an emphasis on devices, as they're less straightforward. | |
For general information on [files](file.html) and [filing systems](filesystem.html), classes specifically for persistant storage, see the respective documentation. This chapter covers the abstract API, with an emphasis on devices, as they're less straightforward. |
### Relationship of FileHandle to other APIs | ||
|
||
Mbed `FileHandle`s can be used directly, but they are often manipulated via POSIX or C/C++ APIs. The layering is that stdio calls taking `FILE *stream` call the POSIX APIs taking `int fd`, which call methods on `FileHandle` objects. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may help to add a picture here. Maybe something like this?
C stdio API: FILE *fopen(const char *path, const char *mode);
|
v
POSIX API: int open(const char *path, int flags);
|
v
Mbed OS C++ FileHandle API: int FileHandle::open(const char *path, int flags);
|
v
Mbed OS C++ File: int File::open(FileSystem *fs, const char *path, int flags);
|
v
Mbed OS C++ FileSystem: int FileSystem::file_open(mbed::fs_file_t *f, const char *path, int flags);
|
v
Mbed OS C++ FATFileSystem: int FATFileSystem::file_open(mbed::fs_file_t *f, const char *path, int flags);
|
v
ChanFS FAT File System: FRESULT f_open (FIL* fp, const TCHAR* path, BYTE mode);
Ok I may have gotten a bit carried away. Feel free to cut that off where it's relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably helps. Think we fall out of scope after File
- a concrete implementation of FileHandle
.
And open
is a special case. Maybe stick to a core call.
|
||
Important notes on sigio: | ||
|
||
* The sigio may be issued from interrupt context. You cannot portably issue `read` or `write` calls directly from this callback, so you should queue an `Event` or wake a thread to perform the `read` or `write`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we link to event queue documentation?
|
||
The `FileHandle` abstraction permits stream-handling code to be device-independent. For example the console input and output streams used for C's `stdin` and `stdout` can be retargeted to something other than the default serial port via the `FileHandle` API, and `ATCmdParser` and the PPP connection to lwIP work on abstract `FileHandle` pointers. | ||
|
||
Exactly which operations a `FileHandle` supports will depend on the underlying device, and will in turn restrict what applications it is suitable for. For example, a database application might require random-access and `seek`, but this may not be available on a limited filesystem, and certainly not on a stream device. Only a `File` on a full `FileSystem` such as `FATFileSystem` would generally implement the entire API. Specialised devices may have particular limitations or behavior, which they should document, and which would limit their general utility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shoud we note that if not supported, implementations should return ENOSYS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will note that "not supported" errors should be indicated - sometimes there's a more specific code, eg ESPIPE
for invalid seek
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
### Related content | ||
|
||
- [File](file.html). | ||
- [FileSystem](filesystem.html). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add UARTSerial here?
Eventually it would be nice to make a "implementation" list section similar to the blockdevices. Though we're probably not there yet with a single non-filesystem implementation 😆
|
||
### Stream-derived FileHandles | ||
|
||
Note that `FileHandle` implementations derived from `Stream`, such as `Serial`, have various limitations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does kinda come out of no where. Should we describe the Stream class?
Note that `FileHandle` implementations derived from `Stream`, such as `Serial`, have various limitations: | |
`Stream` is a legacy class that provides an abstract interface for streams similar to the `FileHandle` class. The difference is that the `Stream` API is built around the `getc` and `putc` set of functions, whereas `FileHandle` is build around `read` and `write`. This makes implementations simpler but limits what is possible with the API. Because of this, the FileHandle API is suggested API for new designs. | |
Note that `FileHandle` implementations derived from `Stream`, such as `Serial`, have various limitations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. Will add an introductory paragraph along those lines.
What's the status of this? |
Reworked incorporating @geky's feedback, and splitting into separate FileHandle and poll pages. Notes:
|
Edit file, mostly for consistent person across docs and international spelling.
Edit file, mostly for active voice.
Thanks for the hard work 👍 Please review my edits to make sure I didn't accidentally change the meaning of anything. |
Gah, just pressed "Cancel review" to close the "review comment" box and deleted all my pending review comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for edits - good stuff. Just a couple of points that have ended up not quite right.
docs/api/platform/Poll.md
Outdated
|
||
As per POSIX, ordinary files always indicate readable and writable via poll. | ||
Ordinary POSIX files are readable and writable using poll. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant "As per the POSIX specification, poll
always indicates that ordinary files (as opposed to devices) are readable and writable".
docs/api/platform/FileHandle.md
Outdated
|
||
The `FileHandle` abstraction permits stream-handling code to be device-independent. For example the console input and output streams used for C's `stdin` and `stdout` can be retargeted to something other than the default serial port via the `FileHandle` API, and `ATCmdParser` and the PPP connection to lwIP work on abstract `FileHandle` pointers. | ||
The `FileHandle` abstraction permits stream-handling code to be device-independent. For example, you can use the the `FileHandle` API to retarget the console input and output streams used for C's `stdin` and `stdout` to something other than the default serial port. You can also retarget `ATCmdParser` and the PPP connection to lwIP work on abstract `FileHandle` pointers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last sentence doesn't want the "You can also retarget".
After than, not sure if it flows right. This is giving 3 examples of device-independence
- Console retargetting stuff
- ATCmdParser
- PPP connection to lwIP
Maybe rework to:
The
FileHandle
abstraction permits stream-handling code to be device-independent, rather than tied to a specific device like a serial port. Examples of such code in Mbed OS are:
- the console input and output streams (
stdin
andstdout
)- the
ATCmdParser
helper- the PPP connection to lwIP
docs/api/platform/FileHandle.md
Outdated
|
||
### Relationship of FileHandle to other APIs | ||
|
||
Mbed `FileHandle`s can be used directly, but they are often manipulated via POSIX or C/C++ APIs. The layering is that stdio calls taking `FILE *stream` call the POSIX APIs taking `int fd`, which call methods on `FileHandle` objects. | ||
You can use `FileHandle` directly, but POSX or C/C++ APIs often manipulate it. Stdio calls taking `FILE *stream` call the POSIX APIs taking `int fd`, which call methods on `FileHandle` objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
POSX->POSIX
How about "You can use a FileHandle
directly, or you can use standard POSIX or C/C++ APIs to manipulate it".
docs/api/platform/FileHandle.md
Outdated
The standard POSIX function `int fileno(FILE *stream)` may be available to map from `FILE` to file descriptor, depending on the toolchain and C library in use - it is not useable in fully-portable Mbed OS code. | ||
|
||
Given those limitations on mapping, if code needs to access the lower levels, a lower-level open call should be used, so the lower-level handle is known, then that is bound to the higher level(s). | ||
Given those limitations on mapping, if code needs to access the lower levels, use a lower-level open call, so the lower-level handle is known. Then, that is bound to the higher level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is dangling a bit with the "currently no call to map from POSIX fd to FileHandle" paragraph gone.
Change the intro to "As it is not possible to map from higher levels to lower levels,".
Final sentence should be imperative to match previous. "Then bind that to the higher level."
|
||
Ordinary files do not generate sigio callbacks, because they are always readable and writable, so never become readable or writable. | ||
Ordinary files do not generate sigio callbacks because they are always readable and writable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the last comma clause was necessary to spell out the logic, but maybe it was going too far. Up to you.
- The sigio callback is only guaranteed when a
FileHandle
becomes readable or writable - Ordinary files are always readable and writable
- Therefore they never become readable or writable
- Therefore they never generate a sigio callback.
Update edits with responses from comments.
Update file with corrections to my edits from comments.
Apply changes from PR #745 to 5.11 to create API reference for FileHandle to fill gap in documentation.
Apply changes from PR #745 to create poll API reference to fill gap in documentation.
Was asked to do
poll
documentation, but can't really do it withoutFileHandle
documentation.First attempt at a combined document. Examples will need extracting.