search

CShark-Hub / Mako-IoT.Home

🏠 The landing page for MakoIoT project.
Other
0 stars 0 forks source link

Only a single IDeviceStartBehavior is executed even if multiple have been registered #63

Closed CoryCharlton closed 1 year ago

CoryCharlton commented 1 year ago

I implemented a IDeviceStartBehavior to initialize display, gpio, etc. and it stopped working when I went to check out Mako-IoT.Device.Services.ConfigurationManager.

Looking at https://github.com/CShark-Hub/Mako-IoT.Device/blob/4c1002331213f9aa75084e9cd66fb1be1d174c10/src/MakoIoT.Device/IoTDevice.cs#L22 it's pretty obvious now as only a single IDeviceStartBehavior is retrieved and executed.

For my particular use case I'll just add custom ConfigurationManager like behavior to my existing IDeviceStartBehavior so I can have feedback on the display during the process.

However this got me wondering if the IoTDevice class should be using ServiceProvider.GetServices(typeof(IDeviceStartBehavior)) to get all the registrations and then do one of the following:

  1. Execute all the IDeviceStartBehaviors
  2. Throw an exception explaining that only a single IDeviceStartBehavior is supported.

I'm happy to implement either change and send over a PR but wanted confirmation on which direction is preferred. Personally I'm leaning toward option 2 as the execution order of multiple IDeviceStartBehaviors is non-deterministic.

jazzman55 commented 1 year ago

Hey @CoryCharlton! Initially the plan was to execute multiple behaviors, but you raised a valid point. Unless we have a way to decide on the order, let's go for option 2.

torbacz commented 1 year ago

@CoryCharlton Merged, NuGet should be available shortly.

CoryCharlton commented 1 year ago

Hey @CoryCharlton! Initially the plan was to execute multiple behaviors, but you raised a valid point. Unless we have a way to decide on the order, let's go for option 2.

More thought would be required to correctly implement some sort of ordering.

At first glance controlling the order itself seems relatively straight forward (ie: add a Priority property to IDeviceStartBehavior). There still could be collisions with a simple solution like that. However the larger issue would be that something like ConfigurationManager doesn't have any context to know if it needs to run before or after user supplied IDeviceStartBehavior implementations.

A second option could be requiring a configuration option be registered if more than one IDeviceStartBehavior is found. The configuration option could be an ArrayList that must include all IDeviceStartBehavior.

Long story short I implemented the simple solution that resolves the current issue user wondering "why didn't my IDeviceStartBehavior run".

I will open a feature request for supporting multiple IDeviceStartBehavior with a deterministic execution order so the team can determine if this is something that some be implemented and provide feedback on possible implementation methods.