[all] support for OT Posix node with RCP using realtime UART V2#769
[all] support for OT Posix node with RCP using realtime UART V2#769EskoDijk wants to merge 1 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the simulation environment by introducing a real-time mode and enabling the integration of OpenThread Posix nodes with RCPs via real-time UART. These changes allow for more realistic and complex simulation scenarios, particularly for future OpenThread Border Router deployments. The underlying dispatcher and node communication mechanisms have been updated to seamlessly support both real-time and virtual-time interactions, improving the robustness and flexibility of the simulation platform. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
43bd5c4 to
fac8b06
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality by adding support for OT Posix nodes with RCP in a real-time simulation. The changes are extensive and well-structured, including a new real-time simulation loop, dual support for virtual and real-time UART, and the necessary build scripts, tests, and documentation updates. Several refactorings, such as in the simulation loop and UART data processing, have also improved code quality and maintainability. The implementation appears solid, but I have a couple of suggestions to improve code clarity by replacing magic numbers with named constants in the new real-time simulation logic.
Note: Security Review did not run due to the size of the PR.
8b66aea to
c0176d4
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive support for running OT Posix nodes with RCP using real-time UART communication, enhancing the simulation's realism and flexibility through significant refactoring and new features across various packages. However, it also introduces security risks related to arbitrary executable execution and insecure URL construction. Specifically, the exe command allows users to specify arbitrary paths for executables, and the Spinel URL is constructed without proper escaping of user-controlled components. These issues should be addressed by implementing path validation for executable paths and proper URL encoding for user-controlled components in the Spinel URL.
c0176d4 to
9afddc6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality to support real-time simulation with RCP nodes. The changes are extensive, touching the dispatcher, simulation core, node implementation, build scripts, and tests. The refactoring to support both virtual-time and real-time simulation modes is well-executed, and the introduction of waitForSimulation simplifies the event processing logic considerably. The code quality is high, and the new features are accompanied by corresponding tests. I have a couple of minor suggestions to improve code clarity and maintainability.
9afddc6 to
846fb79
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality by adding support for real-time simulation with OT Posix RCP nodes. The changes are extensive, touching the dispatcher, simulation core, node implementation, build scripts, and tests. The implementation appears robust and well-designed, with a new real-time dispatcher loop, real-time UART handling over stdio, and several refactorings that improve code clarity and maintainability, such as the introduction of waitForSimulation. The new feature is also accompanied by corresponding documentation updates and new unit tests. I have one minor suggestion regarding a comment.
80273e0 to
927735e
Compare
This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds like virtual-time simulation. A method waitForSimulation() is introduced to avoid the need for custom simulation event processing loops at different places in the code. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. Close openthread#769 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
bc6b961 to
bba807a
Compare
francoismichel
left a comment
There was a problem hiding this comment.
Thank you @EskoDijk for putting all this together!
Also thanks @francoismichel for showing it's possible! I'll start soon with adding the OTBR on top of this, there's the Github issue #739 for tracking the work (needs updating also). Feel free to comment there as well. |
bba807a to
3019eda
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality by adding support for real-time simulations and OT Posix nodes with RCPs. The changes are extensive, touching the simulation core, dispatcher, command-line interface, and node implementation. The introduction of a real-time simulation mode with a new time-keeping mechanism is a major enhancement. The code is well-structured, and the changes include corresponding updates to documentation and new unit tests to cover the new features. Overall, this is a solid contribution. I have found one issue in the UART data processing logic that could be improved for robustness.
|
Last push (May 11) corrects code that's used to suppress displaying of the final 'exit' CLI command that's sent when deleting a node. |
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
The usage of global vars and 'extern' in the RFSIM platform was hard to disentangle. This fixes the issue by placing all 'extern' references in the header file along with comments as to what these variables do. This way, all the relevant sources have access to these global vars.
797630e to
fdc1e19
Compare
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
The usage of global vars and 'extern' in the RFSIM platform was hard to disentangle. This fixes the issue by placing all 'extern' references in the header file along with comments as to what these variables do. This way, all the relevant sources have access to these global vars.
fdc1e19 to
d20acf0
Compare
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
The usage of global vars and 'extern' in the RFSIM platform was hard to disentangle. This fixes the issue by placing all 'extern' references in the header file along with comments as to what these variables do. This way, all the relevant sources have access to these global vars.
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
d20acf0 to
567d555
Compare
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
567d555 to
92ed787
Compare
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
92ed787 to
6dc17b5
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Radio Co-Processor (RCP) nodes and a new real-time simulation mode. Key updates include the implementation of a RunRealtime dispatcher loop, support for forking RCP processes using PTY/UART communication, and enhancements to the CLI and documentation. The PR also refines node lifecycle management with distinct exit initiation and finalization steps and utilizes atomic types for improved concurrency. Feedback highlights a potential busy-wait issue in the RunRealtime loop when nodes are active but no events are scheduled, which could lead to excessive CPU consumption.
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
6dc17b5 to
9a4a3af
Compare
|
Last push contains updates based on AI review comments Gemini/Claude. |
|
@jwhui @francoismichel A number of updates/fixes were made based on MM feedback and based on testing with an OTBR too. This PR looks ready now! |
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
9a4a3af to
cbf9510
Compare
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
cbf9510 to
394db7c
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements support for Radio Co-Processor (RCP) nodes and a real-time simulation mode. Significant updates include refactoring the dispatcher and simulation logic for real-time event handling, adding build scripts and platform code for RCP and Posix host executables, and expanding the testing and logging infrastructure. Review feedback suggests adding documentation for specific magic numbers in log parsing and providing context for reduced strictness in certain unit test assertions.
| n := logIdx[1] + 1 | ||
| linePart := line[n:] | ||
| moduleColonOffset := strings.Index(linePart, ": ") | ||
| if moduleColonOffset >= 8 && moduleColonOffset <= 28 { |
There was a problem hiding this comment.
good point, will add the context
| # check number of partitions | ||
| ns2_pars = ns2.partitions() | ||
| self.assertEqual(1, len(ns2_pars)) | ||
| self.assertTrue(len(ns2_pars) <= 2) |
There was a problem hiding this comment.
There was a problem hiding this comment.
reason here: we don't want the test to fail on the number of partitions != 1 here, as this isn't the main goal of the test. Sometimes partition forming just takes time here, because different random seed is used every time for the RF model.
…ead#769) This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker. The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds - like virtual-time simulation is. Deletion of nodes is updated in real-time mode: it is ongoing in the background in a separate goroutine, to ensure the simulation doesn't halt upon node deletion and that all final log messages of the exiting node are captured also. A method waitForSimulation() is introduced to be more clear when command processing needs to wait for simulation events to be processed first. It simplifies the code in expectEvent() and expectLine() functions also. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug. The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be echo. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional. The RFSIM radio C code is updated to avoid going to sleep when there are still OTNS events in the queue, received via the Unix socket. This ensures faster overall processing with less events being sent, relevant for nodes to keep up in real-time mode. Python unit tests are extended with a more strict error checking, to ensure that unexpected errors from OTNS cause the unit tests to fail. Finally, previous build errors for the rcp target (Issue openthread#758) are now resolved. Close openthread#758 Co-authored-by: François Michel <francoismichel.dev@gmail.com>
This adds support for running a standard OT Posix node in a realtime simulation, where the Posix process performs realtime SPINEL/HDLC UART communication with an RCP process. Support for RCPs is the basis for upcoming features like including real OTBRs in a simulation, either running on the local host or running in a Docker.
The dispatcher/simulation core is updated to allow handling of both 'realtime UART' (stdout) based CLI and logging interactions and 'virtual-time UART' interactions by nodes, in the same simulation. The real-time simulation mode now has a different time keeping mechanism that is synced to the host's clock and processes continuously, not based on discrete time periods or different speeds like virtual-time simulation.
A method waitForSimulation() is introduced to avoid the need for custom simulation event processing loops at different places in the code. Also, the queueing and filtering of UART/log output of OT nodes is simplified to make the code better readable and the UART data flow easier to understand and debug.
The CLI (over PTY/UART/...) echo behavior differs for Linux Posix nodes that are controlled via stdin/stdout: there is no 'echo' behavior. Depending on OS, there could be. To handle all possible combinations of platform/node-type, there is a new function 'setupCli' added that autodetects whether echo is used for a node, or not, and whether the CLI connection is functional.
Co-authored-by: François Michel francoismichel.dev@gmail.com