Skip to main content


Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Archived » Eclipse SmartHome » Idea to get the result of a thing configuration update
Idea to get the result of a thing configuration update [message #1709606] Tue, 29 September 2015 14:04 Go to next message
Thomas Hoefer is currently offline Thomas HoeferFriend
Messages: 8
Registered: September 2015
Junior Member
I am currently investigating what is the best approach to get the result whether the thing configuration has been successfully updated or not.

I have considered to introduce an object called ThingConfigurationUpdateResult as result for the operation "handleConfigurationUpdate" in the thing handler:

    ThingConfigurationUpdateResult handleConfigurationUpdate(Map<String, Object> configurationParameters, Locale locale);


The ThingConfigurationUpdateResult object would consist of two attributes:


  • successful : boolean // indicates that the config update was successful or not
  • configurationErrors : Map<String, String> // containing the internationalized configuration error messages (using the provided locale)


So as binding developers you would have to provide the corresponding ThingConfigurationUpdateResult object when overwriting the handleConfigurationUpdate operation.

I would very much appreciate getting your comments for this approach.

Thx...

Best regards

Thomas

Re: Idea to get the result of a thing configuration update [message #1709616 is a reply to message #1709606] Tue, 29 September 2015 15:25 Go to previous messageGo to next message
Chris Jackson is currently offline Chris JacksonFriend
Messages: 256
Registered: December 2013
Senior Member
The problem I see with this is that for many wireless protocols, configuration may take some time - it could be a few milliseconds, or even tens of seconds, or, it could take 1 day... I think that the update method should return quickly, so in cases where the changes need to be communicated over an 'slow' link, a positive response can not be assured.

What is the purpose of this response - ie where does it go? Given that most configuration comes from the user via the REST interface, is the intention to provide this response back to the user via REST? I would very much like to see this, but from previous discussions with Denis this wasn't something that was deemed desirable...

However, if this is meant to provide feedback to the user, I would have liked to have seen a method that can account for updates that may take time. In ZWave/HABmin in OH1, I hold a pending status, so if an update is not yet confirmed, it shows pending, and when the update is confirmed (or rejected) the final update is provided. For anything that takes time, I think this is very useful, and as above, for ZWave battery device configuration, it will for sure take many hours for this to be updates...

Just food for thought Smile

Cheers
Chris
Re: Idea to get the result of a thing configuration update [message #1709636 is a reply to message #1709616] Tue, 29 September 2015 18:24 Go to previous messageGo to next message
Thomas Hoefer is currently offline Thomas HoeferFriend
Messages: 8
Registered: September 2015
Junior Member
Hi Chris,

thanks for your thoughts; sry that I have not described my concrete requirement exactly.

Yes, among others the response is to be provided for the user. In our team we are currently developing a generic configuration client for the configuration of any things / rules / etc. In this configuration client we are modelling configuration flows for things / rules / etc that consist of one or multiple configuration steps. Each step is responsible for the configuration of a set of configuration parameters. After each step the new configuration parameters have to be applied. For this reason I would like to get the information if the new configuration parameters are valid or rather correct. If they are incorrect then I would like to propagate this information back to the user...

I am not sure if I understood you right:
Quote:

However, if this is meant to provide feedback to the user, I would have liked to have seen a method that can account for updates that may take time.

Would you prefer to have an enum (e.g. SUCCESSFUL, PENDING, ERROR) in the ThingConfigurationUpdateResult object instead of the boolean parameter? If the method returns PENDING due to a long-running configuration update then the binding developer could publish the final ThingConfigurationUpdateResult in a separated launched thread via a dedicated topic, e.g. smarthome/things/{thingUID}/configuration

The only issue I see here is the association between corresponding configuration step of our generic configuration client and the ThingConfigurationUpdateResult object sent via a topic. Maybe this can be solved by injecting an additional parameter, e.g. context, into the handleConfigurationUpdate operation...

Best regards

Thomas
Re: Idea to get the result of a thing configuration update [message #1709843 is a reply to message #1709636] Thu, 01 October 2015 13:11 Go to previous messageGo to next message
Kai Kreuzer is currently offline Kai KreuzerFriend
Messages: 673
Registered: December 2011
Senior Member
The general question to me is: Should the handler be in a position to reject configuration updates? So far I considered the handleConfigurationUpdate method to be similar to initialize - it should
- be non-blocking, i.e. do no communication that might potentially lead to a timeout
- accept what it finds as a configuration and if it is unhappy about it, set the ThingStatus accordingly
- do any device communication asynchronously in a separate job and update the ThingStatus if anything fails

Because of this expected behavior, I doubt that ANY return value makes sense; the result is usually only available asynchronously, but then it is "too late" as it is rather a post- and not a pre-validation.
If a post-validation is acceptable, we could discuss how we can notify about results. I would prefer, if this is a concept that also covers the case that some configuration becomes invalid by itself (e.g. because credentials are no longer accepted). I could imagine that the notification infrastructure discussed at https://bugs.eclipse.org/bugs/show_bug.cgi?id=434010 might be a good solution for this. It is something that UIs could access to display all existing problems in configurations and have a direct link to the entity in question (the notification context).

Regards,
Kai
Re: Idea to get the result of a thing configuration update [message #1710667 is a reply to message #1709843] Thu, 08 October 2015 09:37 Go to previous messageGo to next message
Thomas Hoefer is currently offline Thomas HoeferFriend
Messages: 8
Registered: September 2015
Junior Member
Hi all,

unfortunately the Notifications concept does not meet the requirements for our generic config flow. For this reason I would like to propose another concept. I would very much appreciate to get your feedback / remarks / concerns.

In this concept the config core bundle will provide a new interface called ConfigValidator.
public interface ConfigValidator {
     /* config error consists of parameterName:String (the affected config parameter) and a message:String (the internationalized error message) */
    Collection<ConfigError> validate(String entityId, Locale locale)

    /* returns true if this config validator can validate the entity to be identified by the given entityId (e.g. thing uid) */
    boolean canValidate(String entityId);
}

For each entity (thing, rule, ...) that contains a configuration such a config validator can be registered as service. So in case of things the config validator is registered as service after the thing handler has been registered. For this reason the BaseThingHandler also implements the ConfigValidator interface. Then binding developers can overwrite the validate operation for their own validation.

Furthermore the config core bundle will provide a new service called ConfigValidationService into which all registered config validators are injected. Consumers of the ConfigValidationService can invoke:
public ConfigValidationResult validate(String entityId, Locale locale) 

So this service operations loops over all registered config validators, checks if the current config validator can validate the given entity, if yes invokes the corresponding validate operation of the config validator and puts all retrieved config errors into the config validation result (I could imagine that there could be multiple errors for one config parameter; for this reason the result object is able to hold multiple error messages per parameter; what do you think?).
public class ConfigValidationResult {

    private final SetMultimap<String, String> configErrors = HashMultimap.create();

    public void addConfigError(ConfigError configError) {
        configErrors.put(configError.parameterName, configError.message);
    }

    public void addConfigErrors(Collection<ConfigError> configErrors) {
        for (ConfigError configError : configErrors) {
            addConfigError(configError);
        }
    }

    public Collection<String> getConfigErrorMessages(String parameterName) {
        Collection<String> configErrorMessages = configErrors.get(parameterName);
        if (configErrorMessages != null) {
            return Collections.unmodifiableCollection(configErrorMessages);
        }
        return Collections.emptyList();
    }

    public Map<String, Collection<String>> getConfigErrors() {
        return ImmutableMap.copyOf(configErrors.asMap());
    }
}


Additionally there is one config validator registered (ConfigDescriptionValidator) that matches automatically the current configuration with the config description and provides the corresponding config errors if there are any issues. The ConfigDescriptionValidator will also provide an operation where a subset of the configuration can be validated against the config description. This feature can be used e.g. as kind of pre-validation before handleConfigurationUpdate is invoked.

The ThingResource will offer an additional resource so that UIs can also validate the current configuration:
@POST
@Path("/{thingUID}/config/validation")
public Response validateConfiguration(@HeaderParam(HttpHeaders.ACCEPT_LANGUAGE) String language,  @PathParam("thingUID") String thingUID) throws IOException {
    ConfigValidationResult result = configValidationService.validate(thingUID, LocaleUtil.getLocale(language));
    return Response.ok(result.getConfigErrors()).build();
}


Any comment is appreciated. Thx...

Best regards

Thomas
Re: Idea to get the result of a thing configuration update [message #1710834 is a reply to message #1710667] Fri, 09 October 2015 15:30 Go to previous messageGo to next message
Kai Kreuzer is currently offline Kai KreuzerFriend
Messages: 673
Registered: December 2011
Senior Member
Hi Thomas,

Thanks a lot. To me this looks like a not-too-intrusive addition that is also nicely decoupled from specific entity types such as things and thus should be easily reusable.
A few remarks:
- As no entity or configuration is passed into ConfigValidator.validate(), the ConfigDescriptionValidator won't be able to do its work. Instead of this service, maybe a static helper can be offered somewhere to allow validators to easily check against an existing config description?
- Reading "pre-validation" and "provide an operation" on ConfigDescriptionValidator makes me think if this does not at all implement the ConfigValidator interface then? If this is the case, who is supposed to use it and how?
- I don't like the signature of ConfigValidationResult very much. I would not talk about errors here, but rather about validation messages. Do we have to add flags like info/warning/error possibly on these as well? Do we need status codes as well or is free text feedback really enough?
- I would call the REST URI /{thingUID}/config/validate

I would love to also see feedback from others (Chris? Dennis? Markus?) about this, since it is an addition deep in the core framework and APIs.

Thanks,
Kai
Re: Idea to get the result of a thing configuration update [message #1710885 is a reply to message #1710834] Sat, 10 October 2015 10:02 Go to previous messageGo to next message
Chris Jackson is currently offline Chris JacksonFriend
Messages: 256
Registered: December 2013
Senior Member
Looks interesting to me... A few thoughts-:


  • I think it would be good to have an enum status result from ConfigValidationResult so that the status can be clearly indicated in the UI - a text map is not easy for a UI to present - of course it's great to provide the user with detailed information, but in a summary display, it's not useful. I would like to see OK, PENDING, ERROR, WARNING type response as well.
  • It would also be good if there can be an SSE event associated with changes in state. I'm not sure that this is possible with the architecture given that it looks like the configuration is only validated via the REST interface. You could envisage a method in ConfigValidator that is called by the thing when it has updated the configuration that performs a validation immediately, and sends out an SSE event to the UI rather than having to require the UI to poll. This is especially important on 'slow' devices like RPi - in HABmin for OH1 I have a similar system to this, and in order to keep the UI updated, I need to poll every 5 seconds, but on the RPi it's quite slow to respond if you have to keep a lot of devices updated, so it would be preferable to have an event to eliminate this polling.


Chris
Re: Idea to get the result of a thing configuration update [message #1710909 is a reply to message #1710885] Sat, 10 October 2015 19:18 Go to previous messageGo to next message
Thomas Hoefer is currently offline Thomas HoeferFriend
Messages: 8
Registered: September 2015
Junior Member
Hi Kai, Hi Chris,

thank you very much for your feedback. Pls let me comment on your remarks:

Quote:
As no entity or configuration is passed into ConfigValidator.validate(), the ConfigDescriptionValidator won't be able to do its work. Instead of this service, maybe a static helper can be offered somewhere to allow validators to easily check against an existing config description?

Pls check a snippet of my current config description validator implementation:
public class ConfigDescriptionValidator implements ConfigValidator {
...
    private List<ConfigProvider> configProviders = new CopyOnWriteArrayList<>();
    private ConfigDescriptionRegistry configDescriptionRegistry;
    private I18nProvider i18nProvider;

    @Override
    public boolean canValidate(String entityId) {
        return true;
    }
    @Override
    public Collection<ConfigError> validate(String entityId, Locale locale) {
        List<ConfigError> configErrors = new ArrayList<>();
        ValidationData validationData = getValidationData(entityId); // validation data is an internal class of this validator; containing only configuration and its description
        if (validationData != null) {
            Configuration config = validationData.configuration;
            for (ConfigDescriptionParameter configDescriptionParameter : validationData.configDescription
                    .getParameters()) {
                if (configDescriptionParameter.isRequired()) {
                    validateRequired(configDescriptionParameter.getName(), config, configErrors, locale);
                }
                // add more validations here
            }
        }
        return configErrors;
    }

    private ValidationData getValidationData(String entityId) {
        for (ConfigProvider configProvider : configProviders) {
            Configuration configuration = configProvider.getConfiguration(entityId);
            ConfigDescription configDescription = getConfigDescription(entityId, configProvider);
            if (configuration != null && configDescription != null) {
                return new ValidationData(configuration, configDescription);
            }
        }
        return null;
    }
 ...

As you see in the code above I introduced a so-called ConfigProvider interface as well. Each configuration-based entity type can provide such a config provider once. In case of things I have registered a ThingConfigProvider (it is in package org.eclipse.smarthome.core.thing.internal). The signature of the ConfigProvider interface is as follows:
    Configuration getConfiguration(String entityId);
    URI getConfigDescriptionURI(String entityId);


Quote:
Reading "pre-validation" and "provide an operation" on ConfigDescriptionValidator makes me think if this does not at all implement the ConfigValidator interface then? If this is the case, who is supposed to use it and how?

There is an additional operation in ConfigDescriptionValidator having the following code:
public ConfigValidationResult validate(String entityId, Locale locale,
            Map<String, Object> configurationParameters) {
        for (ConfigProvider configProvider : configProviders) {
            ConfigDescription configDescription = getConfigDescription(entityId, configProvider);
            if (configDescription != null) {
                Configuration configuration = new Configuration(configurationParameters);
                Map<String, ConfigDescriptionParameter> map = toMap(configDescription);
                List<ConfigError> configErrors = new ArrayList<>();
                for (String key : configuration.keySet()) {
                    ConfigDescriptionParameter configDescriptionParameter = map.get(key);
                    if (configDescriptionParameter != null) {
                        if (configDescriptionParameter.isRequired()) {
                            validateRequired(configDescriptionParameter.getName(), configuration, configErrors, locale);
                        }
                        // add more validations here
                    }
                }
                return new ConfigValidationResult(configErrors);
            }
        }
        return new ConfigValidationResult();
    }

My idea was to provide both the ConfigValidator and the ConfigDescriptionValidator itself as OSGi service. Then I am using the ConfigDescriptionValidator in the ThingResource for resource @PUT /{thingUID}/config as follows (pls ignore the signature of config validation result; I will comment it below):
try {
            ConfigValidationResult result = configDescriptionValidator.validate(thingUID,
                    LocaleUtil.getLocale(language), configurationParameters);
            if (result.hasConfigErrors()) {
                return Response.status(Status.BAD_REQUEST).entity(result.getConfigErrors()).build();
            }

            thingRegistry.updateConfiguration(new ThingUID(thingUID),
                    convertDoublesToBigDecimal(configurationParameters));
        } catch ....


Quote:
I don't like the signature of ConfigValidationResult very much. I would not talk about errors here, but rather about validation messages. Do we have to add flags like info/warning/error possibly on these as well? Do we need status codes as well or is free text feedback really enough?

That is a good idea. We could introduce a ConfigValidationMessage object having

  • an enum "type" with the possible values INFORMATION, WARNING, ERROR
  • an optional integer status code
  • the internationalized message


Quote:
- I would call the REST URI /{thingUID}/config/validate

I was using validation because in REST APIs nouns are to be used. In theory we have to introduce a validations resource... /rest/validations/configuration/{thinguid} ...

Quote:
I think it would be good to have an enum status result from ConfigValidationResult so that the status can be clearly indicated in the UI - a text map is not easy for a UI to present - of course it's great to provide the user with detailed information, but in a summary display, it's not useful. I would like to see OK, PENDING, ERROR, WARNING type response as well.

Not sure if I understood this point right. Would you like to have an aggregated status result on the main ConfigValidationResult object derived from all listed config validation messages (if yes, who determines this value? the binding / rule developer or automatic calculation?) or would you like to add the enum to the ConfigValidationMessage object as Kai suggested (is PENDING as distinct state desired or is it sufficient having ThingStatus.ONLINE / ThingStatusDetail.CONFIGURATION_PENDING)?

Quote:
It would also be good if there can be an SSE event associated with changes in state [...]

That sounds like a nice feature. I will think about it...

What do you think?

Best regards

Thomas
Re: Idea to get the result of a thing configuration update [message #1711005 is a reply to message #1710909] Mon, 12 October 2015 09:50 Go to previous messageGo to next message
Dennis Nobel is currently offline Dennis NobelFriend
Messages: 166
Registered: September 2014
Senior Member
Hi,

some general comments from my side:

I think we need to distinguish between different things:

1) Simple input parameter validation (required, integer vs boolean vs string, string length, etc). This is something that maybe can be done in the UI. Nevertheless I think we also need it at runtime to guarantee that no binding receives invalid values (see also https://bugs.eclipse.org/bugs/show_bug.cgi?id=465428). This is what I would usually call "parameter validation"
2) Enhanced runtime "validation" of configuration: Check if an IP address or password is correct. Usually this requires device communication and may take some time. An ip address might be valid in terms of the config description (is required and must be valid IP), but might be invalid because there is no device behind this IP. I understand that there is a requirement to give detailed information about which parameters might be wrong and moreover which parameter might not yet be applied for battery-powered devices with detailed information, so that the user can solve the problem. In addition even without a change of the configuration, parameters can become "invalid". So for me this is more a detailed status information, rather than a validation.

If I look at the interface ConfigDescriptionValidator it´s not clear to be if it supports 1) and/or 2). I actually think we need to different concepts. One for simple validation (might not be so important at the moment) and one concept for detailed information about the current configuration status of a thing. So if it should support 2) we should at least rename it.

Regards

Dennis
Re: Idea to get the result of a thing configuration update [message #1711425 is a reply to message #1710909] Thu, 15 October 2015 21:11 Go to previous messageGo to next message
Chris Jackson is currently offline Chris JacksonFriend
Messages: 256
Registered: December 2013
Senior Member
Thomas Hoefer wrote on Sat, 10 October 2015 20:18

Quote:
I think it would be good to have an enum status result from ConfigValidationResult so that the status can be clearly indicated in the UI - a text map is not easy for a UI to present - of course it's great to provide the user with detailed information, but in a summary display, it's not useful. I would like to see OK, PENDING, ERROR, WARNING type response as well.

Not sure if I understood this point right. Would you like to have an aggregated status result on the main ConfigValidationResult object derived from all listed config validation messages (if yes, who determines this value? the binding / rule developer or automatic calculation?) or would you like to add the enum to the ConfigValidationMessage object as Kai suggested (is PENDING as distinct state desired or is it sufficient having ThingStatus.ONLINE / ThingStatusDetail.CONFIGURATION_PENDING)?


No - the idea wouldn't be to aggregate the messages, but to provide a value from the binding - I don't think it would be possible to reliably aggregate the messages (??).

I also don't think this is related to the ThingStatus. ThingStatus is the status of the thing (obviously Smile ) but the purpose of your configuration update (I believe?) is to provide feedback on each configuration update, not just the overall status of the thing...
Re: Idea to get the result of a thing configuration update [message #1711620 is a reply to message #1711425] Sun, 18 October 2015 13:22 Go to previous message
Thomas Hoefer is currently offline Thomas HoeferFriend
Messages: 8
Registered: September 2015
Junior Member
Hi all,

thanks for your useful feedback. In the meanwhile I created a PR to address all of your points... https://github.com/eclipse/smarthome/pull/496

Best regards

Thomas
Previous Topic:org.eclipse.smarthome.core.scriptengine 0.0.0' but it could not be found
Next Topic:Config Description Parameter
Goto Forum:
  


Current Time: Fri Apr 26 06:59:00 GMT 2024

Powered by FUDForum. Page generated in 0.04865 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software

Back to the top