Conversation
Ohisemega
added
the
enhancement
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #564 +/- ##
==========================================
+ Coverage 48.02% 49.65% +1.62%
==========================================
Files 95 97 +2
Lines 7725 7732 +7
Branches 669 657 -12
==========================================
+ Hits 3710 3839 +129
+ Misses 3823 3691 -132
- Partials 192 202 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Remove SharedMemoryController object, it's associated methods and configuration strings from the FrameProcessorController class. Add an inject_EOA() method to the FrameProcessorPlugin to allow the controller inject the EOF object to the SharedMemoryPlugin. Delete deprecated methods from the SharedMemoryController class.
Ohisemega
changed the title
| static void dummy_timer() | ||
| { | ||
| } |
There was a problem hiding this comment.
I think we should consider renaming this as dummy timer suggests the implementation is not fully thought out. We can discuss.
There was a problem hiding this comment.
As discussed at meeting, timer is necessary to allow the reactor to tick and terminate cleanly, so this should be named more clearly.
| std::string plugin_name = config.get_param<std::string>(FrameProcessorController::CONFIG_EOA); | ||
| if (plugins_.count(plugin_name)) { |
There was a problem hiding this comment.
This changes the client API, as injecting an EOA frame now requires the name of the plugin to inject into, where as before it was always injected into the shared memory controller. This probably is not used by many detectors but worth noting.
There was a problem hiding this comment.
As discussed - let's look at putting EOA back in to core application, with it sent to all plugins that are head of chain.
ajgdls
left a comment
ajgdls
left a comment
There was a problem hiding this comment.
Some points of discussion before we fully approve. We should probably check through the source code documentation.
timcnicholls
left a comment
timcnicholls
left a comment
There was a problem hiding this comment.
Looks good. Some comments as per discussion in meeting. Let's also target this PR at a dev_2.0 branch, as suggested in the 2.0 roadmap issue #517
| static void dummy_timer() | ||
| { | ||
| } |
There was a problem hiding this comment.
As discussed at meeting, timer is necessary to allow the reactor to tick and terminate cleanly, so this should be named more clearly.
| std::string plugin_name = config.get_param<std::string>(FrameProcessorController::CONFIG_EOA); | ||
| if (plugins_.count(plugin_name)) { |
There was a problem hiding this comment.
As discussed - let's look at putting EOA back in to core application, with it sent to all plugins that are head of chain.
| * \param[in] frSubscriberString - Endpoint for receiving frame ready notifications. | ||
| */ | ||
| void SharedMemoryPlugin::setupFrameReceiverInterface( | ||
| const std::string& frPublisherString, |
There was a problem hiding this comment.
References to "publisher" and "subscriber" for IPC channels should be renamed to "release" and "ready" for consistency with the meaning of the channel, rather than the type.
| reply.set_param(this->get_name() + '/' + SharedMemoryPlugin::CONFIG_FR_RELEASE, this->frReleaseEndpoint_); | ||
| } | ||
|
|
||
| void SharedMemoryPlugin::status(OdinData::IpcMessage& reply) |
There was a problem hiding this comment.
Be good to add some statistics reporting for e.g. frames received
This branch has not been deployed
3 participants
Fixes #565
This is in-view of odin-data release 2.0. See Issue #517 (Roadmap) for details.