Home » Archived » Eclipse SmartHome » Dynamic configuration information
Dynamic configuration information [message #1694226] |
Sat, 02 May 2015 18:45 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
I can call getConfigDescriptionRegistry.getConfigDescriptions or one of the other methods to get the configuration descriptions for an thing-type. If I wanted to modify these at runtime, and provide my own configuration provider, can I do this? I'm not sure what happens deeper in the system - I assume that only 1 provider can respond to any uri? Or maybe not - if so, how is this managed?
I have two needs -:
1) To be able to add some configuration options into a parameters [options] array. These are only derived at runtime and are system/network dependant, so can't go in the XML.
2) To be able to add additional parameters to the configuration. Again, these are derived at runtime depending on how a device is configured (for example).
I can easily create my own provider and completely avoid using the XML provider, but I'd kind of like to be able to lever off the existing infrastructure and just add to/modify the static configuration rather than have to reimplement everything...
I guess I could (probably!?!) use getConfigDescriptionRegistry.getConfigDescriptions to get the static data, and then create my own provider that appends my data and modified the data that needs to be changed? I would guess it needs a different uri, although I suspect that this might then break something else when the system tries to link thing-types to the configuration provider? This seems a little messy though so maybe there's a better way?
Are there any pointers on how this might be achieved? I'm thinking it might be easier to do it all in my own config provider which is what I'm currently doing (mostly for test and concepts), but I want to work out the best way for a the final implementation...
Cheers
Chris
|
|
| | | |
Re: Dynamic configuration information [message #1696195 is a reply to message #1694470] |
Fri, 22 May 2015 17:34 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Hi Kai,
So, getting back to this issue as once the channel parameters gets merged this is probably the last thing holding back the zwave binding...
I see the following options to allow bindings to provide dynamic updates to config data -:
- Override the XML provider as per my previous PR. This would allow the binding to provide its own XML in either the same, or slightly different formats. I know you're not keen on this...
- As #1 - allow the binding to override the XML reader, but not allow changes to the XML format. I think this should be do-able in a similar way to #1.
- Allow the binding to register a 'secondary' provider that can get the ConfigDescriptions from the XML file and change them.
- Change the ConfigDescriptionRegistry class so that it appends all responses together rather than returning with the first response it gets.
I still favour #1 or #2 - I think this is most flexible, and it's simple to implement (as per my previous PR). I know it means people can create their own file formats, but if your plan was/is to allow custom readers, then this still seems to be to be better than pushing people to do their own thing completely and ending up with lots of different formats...
#3 would be a substantial change to the interface - probably a separate service to allow an 'over-ride' type service that can be called after the XML reader to allow updating of the static data.
#4 is a relatively simple change to append config descriptions from all providers into a single description. It would allow additional dynamic configuration to be added, but doesn't allow the binding to update the information in the XML dynamically, which is what I also need...
My use case for the updating of static data is that I need to define some configuration in the XML (or somewhere - it's static), but the options available need to be dynamic as they are dependant on the network configuration...
I'm happy to look at any of the options above, or others if you have other thoughts - I think this is the last stumbling block for me so I'd quite like to find a way forward
Cheers
Chris
|
|
| | | | |
Re: Dynamic configuration information [message #1697025 is a reply to message #1696839] |
Sun, 31 May 2015 16:15 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Hi Kai,
I've just digested what you've suggested - I like the concept, and also think this should work. However I see one problem that I would like to try and avoid if possible... With this approach, we're splitting the thing-type definition and the config definition into separate files, which makes it a bit of a pain to maintain the files, and we end up with an extra few hundred files in the case of zwave . Also, if/when we add an index, we'd possibly need to account for this split if we're somehow dynamically loading files...
There are a few possible options I can think of to solve this -:
- Add a section to the XML to define a config description template. So we would have <config-description uri=""> followed by <config-description-template uri="">. The reader would then register both URIs but only the 'template' version actually has any description. This is effectively the same as your suggestion, but adding the extra element to allow the description to be incorporated into the same file.
- Change the XML schema so that it can take a choice of <config-description>, <config-description-ref> or <config-description-template> (instead of the two choices currently defined). If the template version is provided, then it adds -template to the end of the uri and registers both URIs - as in #1.
- We leave the elements alone, but in the <config-description> element, add a template="true" attribute - then the same as above.
- Or add your option here
What do you think - there's little difference in all three as far as implementation - it's really just how it looks in the XML and possibly conventions for URIs if they aren't defined...
I will look at a PR for this (hopefully you agree that the descriptions and thing-type needs to be together) and we can discuss the XML...
Cheers
Chris
|
|
| | | | | | | | |
Re: Dynamic configuration information [message #1698735 is a reply to message #1698685] |
Wed, 17 June 2015 14:29 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Hi Kai,
I didn't debug it further as I wasn't sure where to start. As per my earlier post, if you change the uri, then the system uses this uri, which does make some sense (I think?). I assume that whoever designed the config-parameters, and added the uri, intended the uri="xxx", then xxx should be used, not the default uri? This is what is happening anyway, so it's not possible to use the uri="xxx" to add an intermediary.
Chris
|
|
| | | | |
Re: Dynamic configuration information [message #1699687 is a reply to message #1699504] |
Thu, 25 June 2015 19:56 |
Kai Kreuzer Messages: 673 Registered: December 2011 |
Senior Member |
|
|
Ok, here is some more in depth feedback after digging into things:
1. You are right, when providing an config-desc-uri within a thing type, this uri is considered to be used both for the thing type as well as for the config description; so defining a config description (as a template) that is not directly meant to be the one for the thing type where it is embedded in is simply not possible, so my suggestion led to a dead end.
2. Rereading your initial requirements that you want to solve, I actually think that your suggestion (4) from above might be the way to go for adding parameters dynamically besides the statically defined ones, i.e. if you simply implement another ConfigDescriptionProvider that has answers for the same uris, these could be merged with the ones from the XMLCDProvider within the registry. We just would have to rule out that both provide parameters with the same key and we would have to discuss, if we need any ordering of parameters from different providers.
3. Wrt your second requirement, the dynamic options, I agreed with Dennis that this definitely qualifies for a new kind of service, something like a "ParameterOptionProvider" - this is something that we will probably need very often and which should not require implementing a full ConfigDescriptionProvider.
Would you agree with these points? If so, how about starting with implementing (2)? Would you want to give it a go?
Best regards,
Kai
|
|
| | |
Re: Dynamic configuration information [message #1701771 is a reply to message #1699687] |
Wed, 15 July 2015 13:59 |
Kai Kreuzer Messages: 673 Registered: December 2011 |
Senior Member |
|
|
Kai Kreuzer wrote on Thu, 25 June 2015 21:563. Wrt your second requirement, the dynamic options, I agreed with Dennis that this definitely qualifies for a new kind of service, something like a "ParameterOptionProvider" - this is something that we will probably need very often and which should not require implementing a full ConfigDescriptionProvider.
As the first requirement has been implemented by now, it is probably time to also further discuss this one!
I'd suggest to introduce something like
package org.eclipse.smarthome.config.core;
public interface ParameterOptionProvider {
List<ParameterOption> getOptions(URI parameterURI, Locale locale);
Set<URI> getSupportedURIs();
}
The ConfigDescriptionRegistry could then check, whether there is any provider registered for a given URL (which might be just a complete config description ("thing-type:hue:LCT001") or a reference to a specific parameter ("thing-type:hue:LCT001#parameterKey") of such a description.
These options should imho be added to the already statically defined options. Or would you see the need to dynamically delete some? I think adding would be in line with how config descriptions themselves are merged as well.
On the REST API, we could additionally add a sub-resource on thing type config descriptions to directly be able to request parameter options; not sure if we will need that in a first step though. Probably only makes real sense if changes would also be evented, which can not easily be done through the suggested architecture.
Looking forward to your feedback!
Cheers,
Kai
|
|
| | |
Re: Dynamic configuration information [message #1702150 is a reply to message #1702058] |
Mon, 20 July 2015 10:48 |
Kai Kreuzer Messages: 673 Registered: December 2011 |
Senior Member |
|
|
Quote:Maybe you're thinking of a possible use case for this, but I can't currently think of one
My use case was e.g. to populate an option list with Wifi SSIDs while they are being discovered, which could take a while. Especially when we are talking about options for a thing and not a thing-type, this means that you are likely to request them FROM the thing on demand, so that the information might not be available immediately. But nonetheless, I hope that we can treat this case with a low priority...
Quote:I'm not sure I see the need to request the options separate from the configuration / thingType either
According to your last comment, you know see a need to request it separate from the thingType
But this is indeed a good point - currently the config-descriptions are on the thing-type rest endpoint and this is also where we need them: If you create a new thing, all you have is its thing-type, so all meta-data must be available here - potentially also dynamically provided data.
To accomplish the same on things, we might have to add another method to the ParameterOptionProvider like:
List<ParameterOption> getOptions(ThingUID uid, String parameterId, Locale locale);
and this options would have to be made available somewhere (through getConfigDescription() on the Thing? Only on a new REST endpoint?). I am not quite sure yet, what would be the best approach Any suggestions?
Regards,
Kai
|
|
| | |
Re: Dynamic configuration information [message #1703338 is a reply to message #1702429] |
Thu, 30 July 2015 17:17 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Kai Kreuzer wrote on Wed, 22 July 2015 10:44
Which leaves us with Chris' problem that he would like to offer options specific to a certain device at a specific moment. The use case with the WLAN SSIDs is actually also such a case, so I guess sooner or later we need to address this.
Where do we go with this one? It's my next target to hit
So, if we don't want to link this to things (and I totally agree with the rationale), then we need an alternative. Currently, we have configDescriptions to describe the configuration options, but the 'other side' of the interface (the actual setting of the config) is done differently for different classes (I think that's true). For example, in the thingHandler, we have the handleConfigurationUpdate method - maybe in the channelHandler there's a similar method (I've not checked) - I don't know what other classes use the configDescriptions (rules I think?) but I guess every implementation is different?
Why aren't there standard interfaces for handling the various configuration related functions. We could have had (eg) ...
// Handle the updates of any configuration
interface ConfigConsumer() {
void handleConfigurationUpdate(Map<String, Object>);
}
// Handle dynamic configuration updates
interface ConfigDescriptionUpdate() {
List<ParameterOption> getOptions(String parameterId, Locale locale);
}
A thingHandler, or ruleHandler, or whatever wants to implement configuration implements these interfaces. Then, all configuration, both description, dynamic changes, and handling of changes, could all go through a central provider (and even REST interface!).
As far as implementation (and looking specifically at things here - other implementations would be similar): For registration, when the thingHandler is created, a check could be made to see if it implements this/these interfaces, then to register this handler with the configDescriptionProvider (along with it's UID). When the config descriptions are updated, the configDescriptionRegistry can request the updates from the handler if there's one registered.
One thing I'm unsure of here is the use of the UID. For things/channels etc, it seems a good idea to include the thingUID as it makes it a simple lookup in the configProvider (and if you didn't use the thingUID, I'm not sure how you'd link it). However, will all implementations of configDescriptions have a UID - eg I think I read rules will use the same config services - will they have a UID?
I realise I've probably confused matters by suggesting a common interface for handling of config changes as well as just covering the issue at hand, but I thought I'd mention it since it does in some ways seem a shame to have a common config description provider, but no common way of handling configuration updates/changes etc... Anyway, I hope that didn't confuse matters (too much!) - we could ignore the handleConfigurationUpdate side, and just look at the ConfigDescriptionUpdate, which is the current issue at hand...
What do you think? I've not thought this through in massive detail, but if you think this is workable, I'd be happy to look at the ConfigDescriptionUpdate impementation further...
Chris
|
|
| |
Re: Dynamic configuration information [message #1703491 is a reply to message #1703457] |
Sat, 01 August 2015 08:38 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Quote:
Imho the problem is that we currently have config descriptions for thing type UIDs, but not for thing UIDs. So how could we possibly bridge this gap?
I believe the above concept should work (but maybe I miss something)... To try and be clearer -:
We define the following interface-:
// Handle dynamic configuration updates
interface ConfigDescriptionUpdate() {
List<ParameterOption> getOptions(String parameterId, Locale locale);
}
This interface gets implemented by the thingHandler if it wants to support dynamic config options.
In the BaseThingHandlerFactory, when the thing is created (in registerHandler), we check if this interface is implemented by the handler. If it is, then we register this handlers UID with the configDescriptionRegistry. In one view, this is a new registry, but it's linked into the configDescriptionRegistry as in my view we're trying to build a single configDescription registry, that is dynamically customisable for each implementation (you might have a different view - it's not the end of the world to split it - it's just more work to link things together...).
A new method is added to the ConfigDescriptionRegistry -:
void registerUIDUpdata(UID, handler) {
map.add(UID, handler);
}
This keeps a simple map of UID to handler. Additionally, we add a new method -:
ConfigDescription getConfigDescription(UID genericUID, URI uri, Locale locale) {
// Get the config descriptions for this URI
// Get the handler from the map for this UID
// Call the handler to add the dynamic config changes
for(parameter : allParameters) {
parameter.options = handler.getOptions(parameter.Id, locale);
}
}
Then in the REST interface (or anywhere we need the config information), we can call this new method if it's for a concrete thing, or anything else that uses configDescriptions - so long as they have a UID!
This registration has therefore registered a UID to a handler that implements the ConfigDescriptionUpdate interface - no links between things and config description are specifically made but we have a link from configDescriptions to UIDs implementing the interface.
I think this works, and I think it meets the requirement of not linking concrete classes directly to configDescriptions. However, as above, I've not looked at it in detail - only the concept - so I might miss something... One area I'm slightly concerned about is that UIDs may not be unique, or may not exist in some implementations that support configDescriptions?
Chris
|
|
| | | | |
Re: Dynamic configuration information [message #1704102 is a reply to message #1704082] |
Fri, 07 August 2015 10:40 |
Dennis Nobel Messages: 166 Registered: September 2014 |
Senior Member |
|
|
Hi guys,
sorry for late response.
I´m trying to catch up the discussion:
1) Chris proposal for common configuration mechanism
In general it is something we can think about. But this would be major change to the framework and also makes the recent change about the handleConfigurationUpdate obsolete. I think it is also not that easy. The binding configuarion, which is at the moment handled by the OSGi config admin would be a case for another configuration. But here you have to take the lifecycle into account. What happens if the service is not yet present? You would begin to reimplement something like the configuration admin. Because of this we originally decided to not have a common configuration mechanism and just share the description.
2) Having config descriptions for Things
I think this is possible in general. But what you missed is the mapping between the URI and the thing UID. At the moment the thing type has a method getConfigDescriptionURI. We would need the same for the thing unless we assume an implicit mapping here. But question is, if you want to provide a specific ConfigDescription for a thing, how does the developer does it? In most cases it will be a variant of the Thing Type Config Description. So the binding developer has to register a ConfigDescriptionProvider, get the CD from the ThingType, modify it and return it. Possible, but sounds complex.
Moreover if you want to support dynamic option lists, but you don´t have a need to specify a specific thing CD, you are forced to do it, otherwise you don´t have the concrete context. This will make the implementation of dynamic value providers quite hard.
3) Proposal for a third option
I think what would work if we just introduce a context ID for the DynamicOptionProvider. So taking the initial interface and just adding a new parameter:
package org.eclipse.smarthome.config.core;
public interface ParameterOptionProvider {
List<ParameterOption> getOptions(URI parameterURI, ContextID contextID /* or maybe UID contextUID */, Locale locale);
Set<URI> getSupportedURIs();
}
We could introduce a new interface ContextID and UID will inherit from it or just us UID. But the binding id is not a UID for example.
We can have a generic REST endpoint an the UI just have to send the thingUID as context for thing configuration.
For me this seems to be a less invasive solution, than the other two
Regards
Dennis
|
|
|
Re: Dynamic configuration information [message #1704112 is a reply to message #1704102] |
Fri, 07 August 2015 11:30 |
Kai Kreuzer Messages: 673 Registered: December 2011 |
Senior Member |
|
|
Quote:Moreover if you want to support dynamic option lists, but you don´t have a need to specify a specific thing CD, you are forced to do it
I think you misunderstood my proposal here. I was talking about "a system service forwarding config description requests for a thing to its thing type" - so binding developers don't have to do anything here (and yes, this ThingConfigDescriptionProvider would have to use an implicit URI, most likely the ThingUID). To provide dynamic options, you would only have to implement a ParameterOptionProvider and register it for the ThingUID CD URI (maybe this could even be done by the framework automatically, if the ThingHandler implements this interface).
Your option 3 does a similar thing, but now somehow implicitly introduces CD variants, by returning different results for the same URI on different requests. How would you map that to the REST API? /thing-types/<type>/config/<context>? Simply offering /things/<thing>/config looks much cleaner to me.
Regards,
Kai
|
|
| | | | | | | | | | | | | | | | | | | | | |
Re: Dynamic configuration information [message #1711614 is a reply to message #1711569] |
Sun, 18 October 2015 11:24 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
I've now implemented 'something'
I've added a ConfigOptionProvider to the config.core -:
public interface ConfigOptionProvider {
Collection<ParameterOption> getParameterOptions(URI uri, String param, Locale locale);
}
Then in the ConfigDescriptionRegistry I've added new methods -:
public ConfigDescription getConcreteConfigDescription(URI configUri, URI concreteUri);
public ConfigDescription getConcreteConfigDescription(URI configUri, URI concreteUri, Locale locale);
Currently the user would need to statically register the service - I did consider adding something into the ThingManager to do this automatically if the thing handler implements this interface. I think this possible, but it's maybe something for later (I don't need this for zwave at least)
I decided to split the two URIs rather than derive one from the other as it's not simple to do this - you need to remove the ID off the end, but also modify the initial part of the URI. To use Kais example above -:
thing-type:hue:bridge
thing:hue:bridge:1
So to derive the thing from the thing-type, I need to remove the id, and change 'thing-type' to thing. Maybe we could have made some assumptions about the format of thing-type/thing but this becomes another constraint when using the CDs.
This leaves the implementation of the config descriptions and the concrete implementation up to implementation rather than using assumptions and constraining implementation.
The next problem is that (currently) config descriptions are served up through the thing-type REST interface and this is no longer appropriate as we also need to pass in the URI of the thing. I have therefore added a GET thing/{thingUID}/config interface.
This is (IMHO) also nice in that the REST interface for thing configuration is then GET thing/{thingUID}/config and PUT thing/{thingUID}/config.
In the REST interface, the code gets the thing-type CD URI, and passes the thing URI as the concrete implementation -:
getConcreteConfigDescription("thing-type:hue:bridge", "hue:bridge:1");
If this is broadly inline with what you were thinking, I'll create an initial PR. I've not added any tests yet, and also not tested it properly in the zwave binding (it's just hacked in at the moment), but at least you can comment on what I've done...
Chris
|
|
| | | | | |
Re: Dynamic configuration information [message #1711804 is a reply to message #1711802] |
Mon, 19 October 2015 20:31 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Quote:
I do not see any reasons why it should not be possible to implement this as a generic service as neither the CDProviders nor the OptionProviders have any dependency to a specific type of entity.
Absolutely - and this is what I've done. The provider is absolutely type agnostic and the implementation of the provider is not type dependant.
However, as I mentioned above, we are now working on a concrete implementation, so in the REST interface (or somewhere!) we need to pass (for example) the thing URI, and the thing-type URI since the resulting ConfigDescription is a combination of the two. Or, we perform a conversion between them but here we would need to constrain the way CDs and concrete URIs are defined so that there's a standard conversion between them.
To me, there are 3 options -:
- A generic REST interface, and have the conversion between CD URI and concrete URI performed in the UI (and the UI passes both the type, and the concrete URI).
- A generic implementation, but a specific REST interface that knows how to convert the CD URI and concrete URI for the specific data type (and the UI passes just the concrete URI)
- A generic implementation and generic REST, but somehow constraining the conversion from CD URI to concrete URI (and the UI passes just the concrete URI)
I chose the second option as I think requiring the UI to know how to do the conversion is wrong, and constraining the URIs also doesn't seem like a good idea.
Or... Maybe I misunderstand something and am missing the point somewhere?
|
|
| | | |
Re: Dynamic configuration information [message #1711815 is a reply to message #1711811] |
Mon, 19 October 2015 21:30 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Quote:
No additional REST interface
But the /config-description REST interface doesn't exist now does it? So you mean you want this interface added but no other interface? (sorry - confused!)
Quote:
A default implementation
So how does this actually work? In the REST interface, we pass the thing URI (or another concrete URI). In the /config-description REST interface, it detects the URI is a thing (somehow?) and gets the thing-type URI (which can not currently be derived from the thing URI).
Quote:
Moreover we will introduce the interface "ConfigOptionProvider"
This is what I've implemented...
Quote:
Yes you have to transform the URI, but this shouldn´t be a big deal.
How do you propose to do this? You need to derive the type from the concrete URI which I don't think is possible at the moment without calling thing.getConfigURI (for things at least). Do we now assume that the type and concrete URI are directly derivable?
So, we have to have a system that can take the thing URI (thing:hue:bridge:1) and convert to the thing-type URI (thing-type:hue:bridge). I agree that this isn't difficult, but it is not how it's currently implemented. Do you want to define a class within the REST interface that knows these conversions - again, this is what I've basically done, I've just used a specific REST API.
You and Kai are obviously discussing this, but I'm obviously not clear on exactly how you see this implemented. Maybe you should review what I've implemented as I think it's 90% the same as you want, it's just the REST API that's different (I hope ).
Chris
|
|
| | |
Re: Dynamic configuration information [message #1712108 is a reply to message #1712056] |
Wed, 21 October 2015 10:50 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Quote:
The structure of a Thing UID always contains the Thing Type UID. So we can grep it without making assumptions
Yes, I know, but my point was that if you are making a completely generic REST interface, then you can't make this assumption since to do this you already know you're dealing with a thing. What you are (I think?) actually saying is that there is only a simgle REST endpoint, but the implementation gets programmed to know about things, channels, rules (or anything else that might use CDs) - as per my option 2 below. This isn't quite the definition of generic that I had in mind since the implementation still has to know what's happening.
Quote:
But the REST Api don´t have to be thing specific, so I would really prefer to just give in the Thing UID and internally delegating it to the Thing Type CD, if no other ConfigDescription exists.
This is what I've done, but something still needs to convert the thing to thing type...
Maybe I try and explain in psuedo code...
Option 1:
// Reset API for GET /thing/ThingUID/config
getThingConfig(thingURI) {
thingTypeURI = thingRegistry.getThing(thingURI).getConfigURI();
ConfigDescription cd = configRegistry.getConcreteConfigDescription(thingTypeURI, thingURI);
return cd; // REST response
}
This would then get repeated for the different types of data using the CD (rules, channels, things....) this different REST endpoint.
Option 2 -:
// Reset API for GET /config-description/UID
getGenericConfig(genericURI) {
// Detect the type of the URI
if(type of URI is thing) {
typeURI = thingRegistry.getThing(genericURI).getConfigURI();
}
else if(type of URI is channel) {
typeURI = channelRegistry.getChannel(genericURI).getConfigURI();
}
// repeated for other types
....
ConfigDescription cd = configRegistry.getConcreteConfigDescription(typeURI, genericURI);
return cd; // REST response
}
This option could be implemented slightly differently than calling the getConfigURI which is available for things - we could make some assumption about the mapping of thingTypes and things which I think Kai suggested, but ultimately the implementation looks pretty similar to the psuedo code...
Or... Do you have an option 3?
What I've currently implemented is option 1, and what I think you're suggesting is option 2? Is that correct or am I still missing something? At the end of the day, the low level implementation (ie the getConcreteConfigDescription method) is the same - it's just the REST endpoint and implementation that's changing.
To me, option 1 is cleaner. In my opinion, neither option is 100% generic and it's cleaner to make the clear distinction rather than make the code implementing the endpoint messy. However, if you think option 2 is better, then I don't mind...
|
|
| | | |
Re: Dynamic configuration information [message #1713897 is a reply to message #1712198] |
Sat, 07 November 2015 17:40 |
Chris Jackson Messages: 256 Registered: December 2013 |
Senior Member |
|
|
Ok, just getting back this one...
I've implemented the ThingConfigDescriptionProvider as suggested, but have a question on how you would prefer the implementation, since I don't think the ConfigOptionProvider service is really necessary now (even though I implemented it first time around).
With the ConfigOptionProvider service we have to register a new service for each thing that wants to implement configurable options - or - we register the service for the binding and let the binding sort out the thing. Either way, it's another service to register - in itself this is not a problem, but when we come to resolve options, we need to loop through all the option providers to find the one we're looking for... This is going to get slow - we already have to loop through the ConfigDescriptionProviders list twice (once to get the thingType, and the other to get the thing).
The other option which I think is simpler is in the ThingConfigDescriptionProvider.getConfigDescription method, we already get the thing - we could easily perform a check to see if a specific interface is implemented (ConfigOptionProvider interface) and if it is, then we call thing.getConfigOption(Parameter p);.
In my opinion, this is simpler for binding implementers (they just need to implement an interface) and it's quicker as we don't have to loop through another list of providers. It doesn't (IMHO) break the requirement not to link the CD with a specific type (ie things in this case) since that is handled within the ThingConfigDescriptionProvider which is by design providing this link.
Does that sound ok? The only downside that I can see is it doesn't allow multiple dynamic option providers. It allows options from the CD providers, and the thing to be merged (so we still have static and dynamic options) but it doesn't allow multiple sources of options for a thing. Personally I can't see why this would be necessary?
I'll update the PR on this basis and we can discuss further since at the end of the day either implementation is ok by me.
Chris
|
|
| | | | | |
Goto Forum:
Current Time: Sun May 19 14:59:15 GMT 2024
Powered by FUDForum. Page generated in 0.12995 seconds
|