-
-
Notifications
You must be signed in to change notification settings - Fork 80
Stdout fixes #409
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
Stdout fixes #409
Conversation
This is part of the Pybricks protocol and isn't Bluetooth only. The USB side has yet to be implemented.
We wouldn't want to wait forever if there is no connection. With stdout, the output is just ignored, but here it makes sense to raise.
This was missed because it did not show in the terminal, but it would test result not match the .exp files.
Ensures we don't send the program stopped status before the program is truly done. In turn, this ensures that a new program can't be started before stdout from the last program is one.
This would cause Bluetooth messages to inefficiently buffer. MicroPython calls mp_hal_stdout_tx_strn at least twice for every print because it makes another call for \r\n. The user may also call it multiple times with short messages. Running the poll hook even when the buffer is not full meant that it would begin to transmit it rather than concatenate them as we intended (this has been in the driver for years). It would also hold up the virtual hub, incrementing the clock twice for every print.
| // Don't raise, just wait for data to clear. | ||
| while (!pbsys_host_tx_is_idle()) { | ||
| MICROPY_EVENT_POLL_HOOK | ||
| MICROPY_VM_HOOK_LOOP; |
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.
Is this really safe? If the host never becomes idle, there is no way to ever interrupt this other than turning off the hub.
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 becomes idle when the host disconnects or when nothing more is scheduled. This is why we want this one. The poll hook can raise and schedule more data.
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 guess that works as long as we have USB stall after a timeout in case USB is still connected but no app is servicing the endpoints.
This splits off a few stdout related commits from #406.
In particular, it fixes pybricks/support#870 to handle unfinished prints or prints without a line break when the program ends.
Also changes the
printcommand to drive the event loop only if it could not write everything. This ensures that subsequent calls can get buffered as intended by the Bluetooth drivers. MicroPython calls this function at least twice per print in order to replace\nwith\r\nso now we can get these out in the same message.Also prepares a small cleanup of the app data module, which was currently Bluetooth only. It needs to go through the host module in order to work with USB, though this has not yet been implemented.