-
Notifications
You must be signed in to change notification settings - Fork 2
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
Register signal analyzer signal/signal analyzer monitor #107
Conversation
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.
See minor comments below
@@ -68,6 +75,8 @@ def load_preselector(preselector_config, module, preselector_class_name): | |||
return ps | |||
|
|||
|
|||
signal_analyzer_monitor = signal_analyzer_monitor |
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.
This line seems to be unnecessary
from .status_monitor import StatusMonitor | ||
|
||
status_registrar = StatusMonitor() | ||
from scos_actions.core.status_monitor import StatusMonitor |
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 think this import is no longer needed since the status_registrar
instance of StatusMonitor
was moved to scos_actions.core
logger.debug("Initializing Signal Analyzer Monitor") | ||
self._signal_analyzer = None | ||
|
||
def register_signal_analyzer(self, sigan): |
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.
Suggest including a type hint here and on the property below to indicate that these expect/return instances of SignalAnalyzerInterface
This adds a new register_signal_analyzer signal to allow signal analyzers to register themselves. In addition, this creates a new core module to allow for the creation of needed core components including the SignalAnalyzerMonitor ans StatusMonitor (moved from status module). Within the hardware module, a register_signal_analyzer handler is connected that uses the SignalAnalyzerMonitor to set the sigan. Currently, the SignalAnalyzerMonitor assumes that only one sigan will be registered and the result is that if multiple are registered, the last one to register will be set as the signal analyzer. In the future, this could be easily modified if there were a need to support multiple sigans. Finally, this increments the version to 8.0.0.