Make chip-tool run on a single event loop #7824
Conversation
cd5ff17 to
4cb6923
Compare
4cb6923 to
719c2bf
Compare
719c2bf to
25232bb
Compare
25232bb to
d163625
Compare
implementation of PlatformManager in _StopEventLoopTask() was too over-eager - this resulted in complaints from TSAN that it was still being accessed by RunEventLoop() when it finally came out of the runloop due to _StopEventLoopTask() signalling its termination. Fix: Move that clean-up to _Shutdown(), where it arguably should have been in the first place. This is nicely symmetric to its initialization in InitChipTask().
| #else // CONFIG_USE_SEPARATE_EVENTLOOP | ||
| err = command->Run(); | ||
| SuccessOrExit(err); | ||
| command->ScheduleWaitForResponse(command->GetWaitDurationInSeconds()); |
There was a problem hiding this comment.
This set of lines highlight how much a mirror image of the other these two approaches are, and capture their essence:
Either you asynchronously schedule work and synchronously wait for the response (separate event loop), or synchronously do work and asynchronously wait for the response (same event loop).
There was a problem hiding this comment.
This set of lines highlight how much a mirror image of the other these two approaches are, and capture their essence:
Either you asynchronously schedule work and synchronously wait for the response (separate event loop), or synchronously do work and asynchronously wait for the response (same event loop).
Could you also add this as a comment in the code (maybe Command.h at the first #if)?
|
@mspang @andy31415 @Damian-Nordic @jmartinez-silabs Please take a look? |
| #else // CONFIG_USE_SEPARATE_EVENTLOOP | ||
| err = command->Run(); | ||
| SuccessOrExit(err); | ||
| command->ScheduleWaitForResponse(command->GetWaitDurationInSeconds()); |
There was a problem hiding this comment.
This set of lines highlight how much a mirror image of the other these two approaches are, and capture their essence:
Either you asynchronously schedule work and synchronously wait for the response (separate event loop), or synchronously do work and asynchronously wait for the response (same event loop).
Could you also add this as a comment in the code (maybe Command.h at the first #if)?
This deserves some significant elaboration? |
Added some. |
* Make it possible to run chip-tool on a single event loop. * Cleaning up the state lock and conditional vars in the POSIX implementation of PlatformManager in _StopEventLoopTask() was too over-eager - this resulted in complaints from TSAN that it was still being accessed by RunEventLoop() when it finally came out of the runloop due to _StopEventLoopTask() signalling its termination. Fix: Move that clean-up to _Shutdown(), where it arguably should have been in the first place. This is nicely symmetric to its initialization in InitChipTask(). * Fix typo in artifact name * Address review comments Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Problem
chip-tool demonstrates how to use
StartEventLoopTask/StopEventLoopTaskto run Matter on a thread different from the "main thread" of the application.It should also demonstrate how to use
RunEventLoopto run Matter on the app main thread. Sort of, since not all implementations ofRunEventLoopdo that.