Home » Archived » Eclipse SmartHome » Idea to get the result of a thing configuration update
|
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 |
Chris Jackson 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
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 |
Thomas Hoefer 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 #1710667 is a reply to message #1709843] |
Thu, 08 October 2015 09:37 |
Thomas Hoefer 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 #1710885 is a reply to message #1710834] |
Sat, 10 October 2015 10:02 |
Chris Jackson 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 |
Thomas Hoefer 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 #1711425 is a reply to message #1710909] |
Thu, 15 October 2015 21:11 |
Chris Jackson 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 ) 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...
|
|
| |
Goto Forum:
Current Time: Fri Apr 26 06:59:00 GMT 2024
Powered by FUDForum. Page generated in 0.04865 seconds
|