Merge elements when getting new composition data

[Rawa]

No description provided.

[Rawa]

Quick fix for #539 possibly this logic should be on a setter in ProvisionedBaseMeshNode.

[philips77]

Thank you for the PR! @roshanrajaratnam is on parental leave so it may take some time before he merges it. Would you mind using your branch until then?

[philips77]

Hi,
I was thinking, that perhaps it would be better, and easier, to just check if nodeFeatures field is set before doing anything in that method, and if so, just returning.
The spec says, that the Composition Data cannot change during a lifetime of a device, so there's no need to do all this logic. It will always give the same result as doing nothing. If not (e.g. on device update, etc), the device should be reset and reprovisioned instead.

[Rawa]

@philips77 Thanks for the response, regarding the changing of elements. Just read up on the Mesh Profile 1.0.1 spec and believe I found what you refereed to under the section Elements:

If the number and structure of elements changes, for example due to a firmware update, the node must be reprovisioned. The Node Removal procedure (see Section 3.10.7) is used when a firmware update is performed that changes the number or structure of elements.

Do you know if this also apply for changes to models within the element?

When writing this I took some inspiration from the iOS library, it also seems to merge the elements in a similar fashion:
https://github.com/NordicSemiconductor/IOS-nRF-Mesh-Library/blob/403433c0aff5f64314fda261e90f1052c9e7853a/nRFMeshProvision/Classes/Mesh%20Model/Node.swift#L840
https://github.com/NordicSemiconductor/IOS-nRF-Mesh-Library/blob/403433c0aff5f64314fda261e90f1052c9e7853a/nRFMeshProvision/Classes/Mesh%20Model/Node.swift#L654

[philips77]

Do you know if this also apply for changes to models within the element?

Yes, it says that the structure of elements can't change without reprovisioning.

Indeed, the iOS library does it a bit differently, but perhaps I can simplify it as well there. I don't remember the reason why I did it this way. Perhaps I was hoping this limitation could be removed.. or for testing?

[Rawa]

Not sure exactly why, I saw it was referenced in this PR:
NordicSemiconductor/IOS-nRF-Mesh-Library#167

It feels a bit wrong just to ignore the element information and just not caring about it. But I understand since the mesh spec does not support this behavior and it is then considered undefined.

I can imagine a user having a mesh, then wanting to add an extra feature (model) to an element thus updating the node with DFU over BLE outside of Mesh. E.g adding the scene server and client to later support scenes. The provisioner could use the VID with the new model information from the new composition data to then derive what features are supported.

The process of unprovisioning, DFU, and then reprovisioning the node and trying to restore the subscribes/publications used would be quite cumbersome to add a feature to a node.

[philips77]

Yes, I know it could have been done differently, but spec explicitly says that it is not possible and reprovisioning is needed. Mind, that when reprovisioned, the device will be assigned a new Unicast Address and all its configuration will be lost.
This is a very strong motivation for using Group addresses for publishing, so that you don't have to reconfigure all the nodes, just the one that got updated.

[Rawa]

Yes, that correct. However, imagine that you have a set of lights that is the majority of your mesh and they receive a new firmware update that adds a new model that would mean that you have basically have to re-do the majority of your mesh. The update processes of unprovisioning, dfu and provisioning can also try to restore the previous settings but the flow is quite cumbersome and error-prone since it requires a lot of connections in series to be established and a lot of messages to be sent to be successful. Anyhow I understand your point, since the spec says the device must be reprovisioned in makes sense that it is what should happen until the spec says otherwise.

Also worth mentioning that it is not only nodeFeatures that can change in composition data. e.g vid, could also change if a new version of firmware is running on the device. I guess it is also up for discussion if it is possible that cid and pid may change as well, I've yet to find the mentioning anything about these fields.

[philips77]

As far as I know nothing can change in the data returned as Page 0 of the Composition Data, including vid, pid, cid, etc.
If the update it fairly small, like a bug fix, and models structure is intact, you may keep the vid. A "proper" version could be available using come Vendor model, etc, But this seems like a hack.

[Rawa]

@philips77 That sounds strange, not allowing for changes to the vid. Then there is no way of knowing what version a device runs by looking at the mesh configuration, and the only way to know is to actually query the device. One can imagine a UI where you would like to show if a list of nodes in a mesh and what firmware updates iare available (even though you have not queried each device for it's current version on before hand). In my opinion It makes sense that pid and cid is fixed but vid to be fixed basically would mean that you are not allowed to upgrade nodes inside mesh, and to be fair the Mesh Profile barely mentions firmware upgrade.

I've tried to look through the mesh profile, to find anything supporting this, but I haven't found anything besides the section mentioned before about Elements' structure not being allowed to change and that the page 0 is mandatory. I've searched for all mentions of "page 0" and "composition data" and read through the sections related, but maybe I've missed something?

[philips77]

Hi,
I talked with our mesh expert. Indeed, Mesh Profile 1.0.1 does not say explicitly about not changing Page 0 of the Composition Data, but this is being clarified in the next version.
The main point of not allowing the CDP0 to be changed is integration with third party eco-sytems. If some other Node gets a CDP0 of a Node, it will not try to repeat "just in case". However, in proprietary eco-systems, you can break this rule if you, for example, update all the nodes.