Skip to content
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

Allow jsonrpc for plugin suspend/resume #1799

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nxtum
Copy link
Contributor

@nxtum nxtum commented Nov 27, 2024

No description provided.

@@ -1010,6 +1009,7 @@ namespace Plugin {
}
else {
result = stateControl->Request(PluginHost::IStateControl::command::RESUME);
NotifySuspendResumeStateChange(callsign, Exchange::Controller::ILifeTime::state::RESUMED);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too early, we can only do so if the state is indeed changed. You know that if you would subscribe to the IStateControl::INotification interface. What if the Suspend or Resume fails ?
Lets have a broader discussion on this if we want this as this has some severe impact..

@@ -979,6 +978,7 @@ namespace Plugin {
}
else {
result = stateControl->Request(PluginHost::IStateControl::command::SUSPEND);
NotifySuspendResumeStateChange(callsign, Exchange::Controller::ILifeTime::state::SUSPENDED);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too early, we can only do so if the state is indeed changed. You know that if you would subscribe to the IStateControl::INotification interface. What if the Suspend or Resume fails ?
Lets have a broader discussion on this if we want this as this has some severe impact..

@pwielders pwielders self-requested a review December 6, 2024 12:20
Copy link
Contributor

@pwielders pwielders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in the call. The issue is that we need to "wait" for the statechange to actually occure (moving to SUSPENDED/RESUMED) and not already issue this at the moment it is being requested as at that time the plugin did not yet "reach" that state.
To accomodate that we discussed putting the subscribng on the IStateControl::INotification in the PluginHost::Server::Service::AcquireInterface(https://github.com/rdkcentral/Thunder/blob/master/Source/Thunder/PluginServer.h#L1479) and the releasing of this interface on the PluginHost::Server::Service::ReleaseInterface(https://github.com/rdkcentral/Thunder/blob/master/Source/Thunder/PluginServer.h#L1563). Than hooping through the _administrator in this class (https://github.com/rdkcentral/Thunder/blob/master/Source/Thunder/PluginServer.h#L1649) you can get to the Controller to send the statechanges for each plugin to subscribers of the controller.

Note that you probably do not need to create a SinkType<> for the Service to receive the notifications as you can just "extend" the Composit (https://github.com/rdkcentral/Thunder/blob/master/Source/Thunder/PluginServer.h#L387) already a member here as _composit (https://github.com/rdkcentral/Thunder/blob/master/Source/Thunder/PluginServer.h#L1650) with the IStateControl::INotification interface to get the required changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants