Closed carlosperate closed 5 years ago
LGTM. The examples are short, simple, and to the point, which I think is perfect for these type of docs.
One minor comment - the docs (already) refer to the uart in one place as a 'bus' - strictly it's not a bus, as it is point to point. Should we call this a port instead, perhaps?
Just a minor note, the micropython.kbd_intr() link goes nowhere, where can I read about the parameters that this method takes? If I search for kbd_intr() in the readthedocs search, this is the only page that appears, so I am left dangling without an answer?
Also, if I search for 'micropython' in readthedocs, there doesn't seem to be a module of that name documented, and while the word appears in the contents page, it doesn't tell me anything about that specific module, what it's methods are (and do) and what their parameters are.
If this is functionality inherited from mainline micropython, is it worth linking over to their docs? Or I though the micropython we have is a special version that only runs on the micro:bit, so cross linking to docs in the core repo could be a bad move?
This is what I get at the REPL when interacting with the V1.0 image:
>>> help(modules)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'modules' is not defined
>>> import micropython
>>> help(micropython)
object <module 'micropython'> is of type module
__name__ -- micropython
const -- <function>
opt_level -- <function>
mem_info -- <function>
qstr_info -- <function>
stack_use -- <function>
heap_lock -- <function>
heap_unlock -- <function>
kbd_intr -- <function>
>>>
I did a readthedocs search for mem_info and nothing was found. So I suspect the micropython module is undocumented on the micro:bit (and without help(modules) there is no way for a user to discover it exists, so it is unicorn dust (magic!))
Shall I log a different ticket for that, would that help to keep this PR focused?
I think it would be bad to link to the core micropython readthedocs, as that page includes other methods that are not supported on the micro:bit, and will confuse our users.
http://docs.micropython.org/en/latest/library/micropython.html?highlight=python
Thanks David!
The upstream MicroPython docs also refer to it as a bus, so I'd be inclined to leave it, but it had never occurred to me that it could be an incorrect use of the term "bus".
The micropython
module and kbd_intr
function are documented in this PR still under work: https://github.com/bbcmicrobit/micropython/pull/578
When they get merged, sphinx will add a link to that specific section to the docs, as the restructured text formatting is already in place here.
Regarding...
Read characters. If nbytes
is specified then read at most that many
bytes, otherwise read as many bytes as possible.
If it returns a bytes object, then it is reading bytes, not characters.
Returning 'as many bytes as possible' - is this capped at the 64 byte internal buffer?
What happens if characters are received at the UART interface but the buffer is full? Are bytes dropped (silently) and/or is there any way to query the uart object to sense that a buffer overflow has occurred? Just mentioning this as I am implementing binary payload transfer at the moment in a project, and this uart object makes error handling in buffer overflow conditions very hard!
Thanks
Thanks for the feedback!
If it returns a bytes object, then it is reading bytes, not characters.
I guess technically a byte and an (ASCII) character are interchangeable in this context, but could lead to confusion. The last commit has edited the working a bit, does that make it more clear?
Returning 'as many bytes as possible' - is this capped at the 64 byte internal buffer?
I would think so, but I am not sure. Perhaps @dpgeorge can confirm.
What happens if characters are received at the UART interface but the buffer is full? Are bytes dropped (silently) and/or is there any way to query the uart object to sense that a buffer overflow has occurred? Just mentioning this as I am implementing binary payload transfer at the moment in a project, and this uart object makes error handling in buffer overflow conditions very hard!
There is no way to detect the data has been dropped, but we do have this open feature request: https://github.com/bbcmicrobit/micropython/issues/504
Yes, that wording is clearer and more correct, thanks!
Returning 'as many bytes as possible' - is this capped at the 64 byte internal buffer?
I would think so, but I am not sure. Perhaps @dpgeorge can confirm.
It's possible to get more than 64 bytes if the data is coming in at the same time as reading. Eg, buffer is nearly full, uart.read()
starts draining it and copying it to the buffer to return, and while it is draining more bytes are added, which will then be drained into the buffer.
@dpgeorge Interesting. So, does this mean that if data is coming in faster than MicroPython can process it and you ask for 'all data' (no limit), it will keep reading and filling a user buffer until memory is exhausted, and then the user script will crash?
does this mean that if data is coming in faster than MicroPython can process it
If that's the case then you need to redesign something in your protocol/device :)
and you ask for 'all data' (no limit)
For UART protocols it's generally not a good idea to ask for "all data" because there is no well defined end to the incoming data.
it will keep reading and filling a user buffer until memory is exhausted, and then the user script will crash?
Yes. It will keep reading and then raise a MemoryError, so the script could catch this and possibly recover.
Sadly the device I am connecting to has it's own defined protocol with long return lines, sometimes longer than 64 characters. Hence another request to have a flag or something that indicates the buffer has overflowed and data has been lost.
Thanks for info. So, reading 'all available data' is not really a useful feature in this API then? Should it be removed?
Thanks for info. So, reading 'all available data' is not really a useful feature in this API then? Should it be removed?
This is the normal behaviour with python streams, so while it might not be a good idea, it can still be useful.
Are there any changes required to this PR, or is it good to be merged?
Ok, good point. With a slower baud-rate it might still work anyway, so worth hanging on to.
LGTM for a merge.
Fixes https://github.com/bbcmicrobit/micropython/issues/368.
Adds info about how to format data to send and receive via UART. It also edits the "Keyboard Interrupt" info to display it as a warning.
@whaleygeek Based on the discussion #368, could you review when you get a chance?