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

Delete value of a "desired" feature property once the actual "property" was set to the same value #698

Open
thjaeckle opened this issue Jun 18, 2020 · 10 comments

Comments

@thjaeckle
Copy link
Member

thjaeckle commented Jun 18, 2020

As part of #696 the goal is to have an empty or even removed "desiredProperties" section in a feature once all desired properties were applied by a device.

This can be done automatically by Ditto whenever a feature property is changed to the same value as an existing desired property with the same access path.

Example:
Given we have our simple room thermostat:

{
  "thingId": "org.eclipse.ditto:my-thermostat-1",
  "policyId": "org.eclipse.ditto:my-thermostat-1",
  "features": {
    "Thermostat": {
      "definition": [ "org.eclipse.ditto:Thermostat:1.0.0" ],
      "properties": {
        "status": {
          "measured-temperature": 20
        },
        "configuration": {
          "target-temperature": 20.0
        }
      }
    }
  }
}

And now e.g. a mobile app sets that the lamp should not be on by setting target-temperature should be adjusted by setting desiredProperties/configuration/target-temperature to 22.0:

{
  "thingId": "org.eclipse.ditto:my-thermostat-1",
  "policyId": "org.eclipse.ditto:my-thermostat-1",
  "features": {
    "Thermostat": {
      "definition": [ "org.eclipse.ditto:Thermostat:1.0.0" ],
      "properties": {
        "status": {
          "measured-temperature": 20
        },
        "configuration": {
          "target-temperature": 20.0
        }
      },
      "desiredProperties": {
        "configuration": {
          "target-temperature": 22.0
        }
      }
    }
  }
}
  • the device registers this change by a received event
  • then, it actually performs the hardware action (changing the target temperature, turning the heating on)
  • when that was a success, it sets its properties/configuration/target-temperature to the applied state: 22.0
  • Ditto will persist this change and automatically remove the desiredProperties/configuration/target-temperature as this has now the same value as the properties/configuration/target-temperature
    • JSON numbers are semantically compared, that e.g. means 22.0 == 22
  • Ditto will also delete empty JSON objects "the way up", including the desiredProperties object if all desired changes were confirmed
{
  "thingId": "org.eclipse.ditto:my-thermostat-1",
  "policyId": "org.eclipse.ditto:my-thermostat-1",
  "features": {
    "Thermostat": {
      "definition": [ "org.eclipse.ditto:Thermostat:1.0.0" ],
      "properties": {
        "status": {
          "measured-temperature": 20
        },
        "configuration": {
          "target-temperature": 22.0
        }
      }
    }
  }
}
@thjaeckle

This comment has been minimized.

@geglock
Copy link
Contributor

geglock commented Jun 25, 2020

I think the automatic cleanup could be unwanted for certain scenarios, e.g. when there is not "full trust" towards the devices or due to audit requirements.
To address this, we could make it optional by having some "desired handling strategy" configuration per Thing, which allows to turn off the automatic cleanup.

In this case the "delta calculation" issue (#699) would be important to avoid redundant desired property transmission.

@thjaeckle
Copy link
Member Author

thjaeckle commented Jun 25, 2020

To address this, we could make it optional by having some "desired handling strategy" configuration per Thing, which allows to turn off the automatic cleanup.

We could add a section for configuring such things, e.g.:

{
  "thingId": "org.eclipse.ditto:my-thermostat-1",
  "policyId": "org.eclipse.ditto:my-thermostat-1",
  "features": {
    "Thermostat": {
      "definition": [ "org.eclipse.ditto:Thermostat:1.0.0" ],
      "properties": {},
      "desired": {},
      "desiredSettings": {
        "desiredDeletionStrategy": [
          "reported-reaches-desired"
        ]
      }
    }
  }
}

@ffendt
Copy link
Contributor

ffendt commented Sep 29, 2020

As far as I understand #696, the desired section is especially meant (and useful) for configuration values (in contrast to measured values). Like in the above example the target-temperature.

If we aim this feature at configuration values, I don't think that we need a check on "same value" at all when deleting the desired value for a property. The user sets a desired configuration value and the device updates the reported configuration value. For me this would mean that the desired value becomes obsolete and can be deleted and thus there's no need to automagically compare the values.

If updates from user and device are expected to cause race conditions for devices, ETags could be used to ensure that the desired/reported state is only updated if the Thing is in the expected state.

@thjaeckle
Copy link
Member Author

True, equality is not a requirement for the cleanup of desired properties.
I think that we somehow should make it configurable nonetheless as the requirements on how the desired state is used could be very different.

@ffendt
Copy link
Contributor

ffendt commented Sep 30, 2020

Maybe it would be sufficient to have the configuration in a way in which Ditto doesn't have to interpret a whole lot into possible use-cases, but just its default use case (e.g. delete desired when new reported) and one configuration that allows users to freely implement behavior for it as they like (e.g. Ditto does nothing when a new reported value comes in). If a user has some special use-case where the configuration value increases slowly towards the desired value, he/she should still be able to implement this behavior on his/her own.

From a Ditto POV I think we shouldn't do hasty assumptions on how the users would use such a feature. If we have the handling configurable, new configurations can still be contributed by users when it becomes obvious that there is a widely used pattern for how they are handling desired/reported states.

@thjaeckle
Copy link
Member Author

one configuration that allows users to freely implement behavior for it as they like

That sounds like dynamic script invocation :) Do you think in the same direction (providing e.g. a JS snippet) or how could this be achieved?

@ffendt
Copy link
Contributor

ffendt commented Sep 30, 2020

Actually I meant that we don't do anything when a new value comes and the user can decide if he/she wants to use our default API to delete a desired value or not.

But I also thought in the direction of providing a separate place for e.g. JS snippets with which the user could define own behavior. That way Ditto wouldn't have to implement different strategies, but just needs like 1-2 default strategies (e.g. delete / not-delete) and another one which references a custom behavior.

@geglock
Copy link
Contributor

geglock commented Sep 30, 2020

I think we don't have to discuss "JS snippets" etc. but just use the standard event notification way: an application could listen to all reported changes and decide on it's on if it wants to delete desired values (with or without comparison of the values).
The "strategy" approach could be just a convenience to support some strategies built-into Ditto.
I.e.
"no strategy" = do nothing - the application can optionally use event notifications and do whatever logic it needs"
"delete desired - strategy" = delete desired property for each updated reported property without comparison
"delete desired if same value is reported - strategy" = ...

@ffendt
Copy link
Contributor

ffendt commented Oct 2, 2020

I agree that it would be nice to have some convenience, but in my opinion we should start as simple as possible. Time will tell how the feature is used and what users expect from it. Then we can think of the places where we need to add convenience.

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

No branches or pull requests

3 participants