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

feature: add registerDeltaInputHandler plugin hook #565

Merged
merged 1 commit into from
Jun 24, 2018

Conversation

tkurki
Copy link
Member

@tkurki tkurki commented Jun 22, 2018

Add addDeltaInputHandler so that plugins can intercept
incoming delta messages and alter them before the server
handles them.

@sbender9
Copy link
Member

sbender9 commented Jun 23, 2018

Going to get a little hairy if more than one plugin does this, right? Specifically when a plugin is stopped.

@sbender9 sbender9 changed the title feature: add addDeltaInputHandler plugin hook feature: add registerDeltaInputHandler plugin hook Jun 23, 2018
@sbender9
Copy link
Member

After playing with this with node red. I think we should maintain a 'chain' of these, this way they can be inserted and removed adhock. Maybe the register function should take a 'priority' parameter, which could be configured by the user. This would allow plugins to stop/restart and the chain could be kept consistent. Still thinking about how this would work and fit into node-red.

https://github.com/SignalK/node-red-embedded/tree/delta-input-handler

@tkurki tkurki force-pushed the addDeltaInputHandler branch from 0007f10 to 38ba87b Compare June 23, 2018 07:25
@tkurki tkurki changed the title feature: add registerDeltaInputHandler plugin hook [WIP] feature: add registerDeltaInputHandler plugin hook Jun 23, 2018
@tkurki
Copy link
Member Author

tkurki commented Jun 23, 2018

Note force push!

Rewritten as a chain of handlers that are added and removed upon plugin start/stop.

TODO:

  • plugin API documentation
  • generate the next functions on chain modification, no need to create new functions on the fly for each and every handled delta, probably really hurts performance.
  • ordering/priority?

Priority should be easy enough to implement, just insert the handler in the correct position and not just push to the end of the chain. That should go into plugin config, as a sibling of enabled? Should the chain be debuggable someplace, show plugin ids in the chain in order? In the plugin config UI somewhere?

@tkurki tkurki force-pushed the addDeltaInputHandler branch from 38ba87b to 2e1348e Compare June 23, 2018 19:04
Add addDeltaInputHandler so that plugins can intercept
incoming delta messages and alter them before the server
handles them.

Handlers are processed in a chain, first started first in chain
order currently.
@tkurki tkurki force-pushed the addDeltaInputHandler branch from 2e1348e to 6b83120 Compare June 23, 2018 19:26
@tkurki
Copy link
Member Author

tkurki commented Jun 23, 2018

Added some documentation and streamlined dispatching so that we don't create new functions per each delta and chain element.

I think we can address the priority issue when we bump against it.

@sbender9 sbender9 merged commit 68276ea into master Jun 24, 2018
@sbender9 sbender9 changed the title [WIP] feature: add registerDeltaInputHandler plugin hook feature: add registerDeltaInputHandler plugin hook Jun 24, 2018
@tkurki tkurki deleted the addDeltaInputHandler branch October 29, 2018 19:12
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