Skip to content

ComPort plugin: possibility to enable/disable by property and events#295

Merged
deinhofer merged 1 commit intomasterfrom
klaus/comport-possibility-to-enable-and-disable
Apr 12, 2019
Merged

ComPort plugin: possibility to enable/disable by property and events#295
deinhofer merged 1 commit intomasterfrom
klaus/comport-possibility-to-enable-and-disable

Conversation

@klues
Copy link
Contributor

@klues klues commented Mar 8, 2019

If a model uses the ComPort plugin but is not used in all cases it's quite annoying to get the error message at model startup COM port not found.

This PR adds the possiblity to disable/enable the ComPort plugin by property and/or events.

final IRuntimeEventListenerPort elpEnablePlugin = new IRuntimeEventListenerPort() {
public void receiveEvent(final String data) {
if (!propEnabled) {
propEnabled = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

is propEnabled describes the state of an enabled (started) plugin, then I would suggest to

  1. start()
  2. propEnabled=true;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

start() does nothing and returns immediately, if propEnabled == false. Therefore propEnabled has to be set to true before starting.


final IRuntimeEventListenerPort elpDisablePlugin = new IRuntimeEventListenerPort() {
public void receiveEvent(final String data) {
if (propEnabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here it would be possible to call stop() first, but it wouldn't make any difference to my mind and it looks somewhat inconsistent with the order in elpEnablePlugin.

@deinhofer
Copy link
Contributor

looks good.
Actually I would prefer a more generic solution for all plugins.

A complex solution for this could be to implement composition of bundle descriptors, so we could define generic functionality.
A simple solution could be to create a plugin which allows to control the lifecycle and properties of any other plugin.

But nevertheless let's merge until we have time to make something more generic.

@klues
Copy link
Contributor Author

klues commented Apr 12, 2019

thanks for the review!
yeah, agree that a generic solution would be great. Maybe a third possibility would be to define some generic functionality for enabling/disabling a plugin in a superclass DeactivatablePlugin and Plugins can derive from this class.

@deinhofer
Copy link
Contributor

thanks for the review!
yeah, agree that a generic solution would be great. Maybe a third possibility would be to define some generic functionality for enabling/disabling a plugin in a superclass DeactivatablePlugin and Plugins can derive from this class.

thanks for the review!
yeah, agree that a generic solution would be great. Maybe a third possibility would be to define some generic functionality for enabling/disabling a plugin in a superclass DeactivatablePlugin and Plugins can derive from this class.

👍

ok, will merge

@klues
Copy link
Contributor Author

klues commented Apr 12, 2019

ok, created an issue for the generic solution: #309

@deinhofer deinhofer merged commit 6cf1879 into master Apr 12, 2019
@deinhofer deinhofer deleted the klaus/comport-possibility-to-enable-and-disable branch April 12, 2019 08:07
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.

2 participants