Skip to content

Implementation of the Shadow Thing mechanism on the device side #36

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

Merged
merged 22 commits into from
Mar 19, 2019
Merged

Implementation of the Shadow Thing mechanism on the device side #36

merged 22 commits into from
Mar 19, 2019

Conversation

ilcato
Copy link
Contributor

@ilcato ilcato commented Feb 19, 2019

It allows the synchronization of property values between cloud and devices, at startup or after a disconnection period, based on synchronization policies configurable at the property level.
It adds a new parameter to the addProperty method that specify the synchronization mode to adopt for the property. This parameter is a callback function that will specify the synchronization logic to use. Standard synchronization logic is available by using the following callbacks:

  • AUTO_SYNC: that compares the cloud timestamp of the last change with the corresponding device timestamp. The property is assigned the value with the higher timestamp.
  • FORCE_CLOUD_SYNC: the property is assigned the value coming from cloud regardless of timestamps and device value.
  • FORCE_DEVICE_SYNC: the device property value is kept. The cloud property value will be updated at the next update cycle.

A custom synchronization logic may be implemented by setting a custom callback function with the following signature: void (*)(ArduinoCloudProperty<T> property); use one of the specific types supported. The property object exposes several methods which will enable the custom logic to select the appropriate value.

Example: assuming a Switch boolean READ_WRITE property is defined the following statement in thingProperties.h will specify an AUTO_SYNC mode:

ArduinoCloud.addProperty(Switch, READWRITE, ON_CHANGE, onSwitchChange, AUTO_SYNC);

@ilcato ilcato requested a review from aentinger February 19, 2019 14:38
@ilcato ilcato requested a review from ubidefeo February 21, 2019 06:43
Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request looks generally good to me, however, some minor changes are recommended.

@mbanzi
Copy link
Contributor

mbanzi commented Feb 21, 2019

I gave a quick look at this, it looks good the only thing I would recommend is that we use a more human friendly way to specify the syncronisation policy i.e. AUTO_SYNC , FORCE_CLOUD_SYNC, FORCE_DEVICE_SYNC could become something lighter like CLOUD_WINS
DEVICE_WINS
MOST_RECENT_WINS. just a thought to make it more understandable

@ilcato
Copy link
Contributor Author

ilcato commented Feb 21, 2019

I gave a quick look at this, it looks good the only thing I would recommend is that we use a more human friendly way to specify the syncronisation policy i.e. AUTO_SYNC , FORCE_CLOUD_SYNC, FORCE_DEVICE_SYNC could become something lighter like CLOUD_WINS
DEVICE_WINS
MOST_RECENT_WINS. just a thought to make it more understandable

Done

Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good by me ;)

@ilcato ilcato removed the request for review from ubidefeo February 21, 2019 17:30
aentinger and others added 2 commits February 27, 2019 08:17
Co-Authored-By: ilcato <ilcato@yahoo.com>
@ilcato
Copy link
Contributor Author

ilcato commented Feb 27, 2019

Mmmh, isn't this the exact opposite of what the patch was supposed to do?

Restored the initial order of the call sequence.
The previous commit was related to the call of the wrong method in the connection managers state machine.

@ilcato ilcato requested review from facchinm and ubidefeo February 27, 2019 17:09
Copy link
Collaborator

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this workflow works better.
As @ilcato noticed we had things a bit wrong when it came to implementing a fallback :)

@ubidefeo
Copy link
Collaborator

If @lxrobotics and @facchinm are good with this I'd merge it into master without yet making a new release

@aentinger
Copy link
Contributor

@ubidefeo I'm good with the changes 👍

@aentinger
Copy link
Contributor

@ubidefeo Just curious - what's the state on the PR? Maybe @facchinm could take a look on it so we can merge it into master and be done with it.

@ubidefeo
Copy link
Collaborator

hi @lxrobotics @ilcato
I am still away but slowly getting back to work remotely.
I think we can merge this in and close the PR.
I can then add a release and get the libs updated

@ubidefeo ubidefeo merged commit 25be97a into arduino-libraries:master Mar 19, 2019
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

Successfully merging this pull request may close these issues.

6 participants