Messages: thread-safety and dynamic memory #418
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* use dynamic memory to allow handling of arbitrary message lenghts. * keep a message buffer for every task so no task ever mangles the message of another task. * every complete line is written to the serial console and moved to a line buffer for sending them through the websocket. * the websocket is always fed complete lines. * make sure to feed only as many lines as possible to the websocket handler, so that no lines are dropped. * lock all MessageOutput state against concurrent access.
the MessageOutput class buffers whole lines of output printed by any task in order to avoid mangling of text. that means we hand over full lines to the HardwareSerial instance, which might be too much in one call to write(buffer, size). we now check the return value of write(buffer, size) and call the function again with the part of the message that could not yet be written by HardwareSerial.
schlimmchen
added a commit
to schlimmchen/OpenDTU-OnBattery
that referenced
this pull request
Dec 29, 2023
this commit re-introduces the changes from hoylabs#418, which were effectively reverted with d494810 (merge commit introducing TaskScheduler). these adjustments are important to guarantee unmangled log messages and more importantly, to guarantee that all messages from a particular component are printed to the web console, which most people use to copy messages from when reporting issues. * use dynamic memory to allow handling of arbitrary message lenghts. * keep a message buffer for every task so no task ever mangles the message of another task. * every complete line is written to the serial console and moved to a line buffer for sending them through the websocket. * the websocket is always fed complete lines. * make sure to feed only as many lines as possible to the websocket handler, so that no lines are dropped. * lock all MessageOutput state against concurrent access. * respect HardwareSerial buffer size: the MessageOutput class buffers whole lines of output printed by any task in order to avoid mangling of text. that means we hand over full lines to the HardwareSerial instance, which might be too much in one call to write(buffer, size). we now check the return value of write(buffer, size) and call the function again with the part of the message that could not yet be written by HardwareSerial.
helgeerbe
pushed a commit
that referenced
this pull request
Dec 30, 2023
this commit re-introduces the changes from #418, which were effectively reverted with d494810 (merge commit introducing TaskScheduler). these adjustments are important to guarantee unmangled log messages and more importantly, to guarantee that all messages from a particular component are printed to the web console, which most people use to copy messages from when reporting issues. * use dynamic memory to allow handling of arbitrary message lenghts. * keep a message buffer for every task so no task ever mangles the message of another task. * every complete line is written to the serial console and moved to a line buffer for sending them through the websocket. * the websocket is always fed complete lines. * make sure to feed only as many lines as possible to the websocket handler, so that no lines are dropped. * lock all MessageOutput state against concurrent access. * respect HardwareSerial buffer size: the MessageOutput class buffers whole lines of output printed by any task in order to avoid mangling of text. that means we hand over full lines to the HardwareSerial instance, which might be too much in one call to write(buffer, size). we now check the return value of write(buffer, size) and call the function again with the part of the message that could not yet be written by HardwareSerial.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am trying to convince tbnobody that my changes proposed in tbnobody#1212 are valuable and do not have a detrimental effect on the project. However, the pull request sits idle.
In the meantime, I am still concerned that the web console message can only be trusted to some extent, as they can still be interleaved in between tasks, and because messages are simply dropped if any component tries to print "too much" messages until the next time MessageOutput's
loop()is called.For that reason, I would like to propose these changes to this project.
I have these changes deployed on my production unit and have no issues whatsoever. Additionally, I sneaked the changes into pre-releases of my fork, and I heard nothing about any issues.