Hi Michal,
> From: Michal Lowas-Rzechonek
> > + uint64 token ImportLocalNode(string config_file)
>
> I am somewhat uncomfortable with passing a file path here. The caller would
> need to create a temporary file, which is a little cumbersome, and might fail if
> the daemon is running on another machine.
>
> Not sure what are the size constraints (if any), but I think it might be better
> to pass the JSON as a string.
Indeed, this has been discussed internally as well, and is still subject to the change you mention. We are still wait8ing for input from all stakeholders, and your preference is noted.
> > + void AddNetKey(object element_path, uint16 destination,
> > + uint16 subnet_index, uint16 net_index, boolean
> update)
> > (...)
> > + void AddAppKey(object element_path, uint16 destination,
> > + uint16 app_index, uint16 net_index, boolean update)
>
> If I understand correctly, these are convenience functions for
> SendWithDeviceKey, so that the application wouldn't need to construct
> access payloads for configuration messages?
>
> If so, I guess we will need more functions like that, ideally to cover all
> messages mentioned in section 4.3.4 of the mesh profile?
These two functions are explicitly separated out from the rest of the section 4.3.4 messages in order to keep the security of the keys in the hands of the daemon. The aoo_index and net_index must already be present in the node's key database, and for these (4 total) Config Client message's, the OTA Access layer message will be composed by the daemon.
The messages where keys are explicitly sent over-the-air are the only messages we anticipate providing specific D-Bus methods for to minimize D-Bus API bloat.
>
> I would suggest a few things:
> - name these functions in the same way as the corresponding message
> - possibly extract these into org.bluez.mesh.ConfigClient1 and
> org.bluez.mesh.HealthClient1 interfaces, so that we don't need to
> name functions "ConfigAppKeyApp", just "AppKeyApp"
> - possibly add corresponding "* Status()" calls to application
> hierarchy; see the question below about DevKeyMessageReceived
We may want to rename the method call to specifically acknowledge that they are Config Client model messages.
>
> > + void ImportRemoteNode(uint16 primary, uint8 count,
> > + array{byte}[16] device_key)
> > +
> > + This method is used by the application to import a remote
> node
> > + that has been provisioned by an external process.
> > +
> > + The primary parameter specifies the unicast address of the
> > + the node being imported.
> > +
> > + The count parameter specifies the number of elements that
> are
> > + assigned to this remote node.
> > +
> > + The device_key parameter is the access layer key that will be
> > + will used to decrypt privledged messages from this remote
> node.
> > +
> > + PossibleErrors:
> > + org.bluez.mesh.Error.InvalidArguments
> > +
>
> > + void DeleteRemoteNode(uint16 primary, uint8 count)
> > (...)
> > + The count parameter specifies the number of elements that
> were
> > + assigned to the remote node.
>
> Do we need the count here? I think the daemon knows that already.
The daemon only explicitly knows the element count of *local* nodes. The composition data (that is important to the Config Client App) of remote nodes is not stored within the daemon space. This is only to load the mappings of Remote Node device keys, and which Unicast addresses that they map to. All additional remote node data is stored as needed by the App. This does however at least keep more than one version of each unicast address from being claimed by more than one remote node.
>
> > + void SendWithDeviceKey(object element_path, uint16 destination,
> > + uint16 net_index, array{byte} data)
> > +
> > + This method is used to send a message originated by a local
> > + model encoded with the device key of the remote node.
> > (...)
> > + void DevKeyMessageReceived(uint16 source, uint16 net_index,
> > + array{byte} data)
> > +
> > + This method is called by meshd daemon when a message
> arrives
> > + addressed to the application, which was sent with the
> remote
> > + node's device key.
>
> Correct me if I'm wrong, but it seems that these two are not required to
> support foundation models?
Initially, they are need to support the Config Client foundational model. It is true that the Config Server of each node is handled internally by the daemon.
The external Config Client App is responsible for:
* deciding *when* to send App and Net key adds and updates
* sending all pub/sub/binding/feature settings
so in this case, the controlling Config Client App will be composing all Config Client messages (except for OTA key messages)
>
> IIRC, requests coming into the daemon from a remote Config Client are
> handled internally and then passed to the application via
> UpdateModelConfiguation() and friends, while outgoing requests are to be
> handled by AddNetKey(), etc.?
>
> On the other hand, nodes might implement vendor models using device
> security, so I guess these functions are meant for this use case?
If not by the current spec revision, I believe it will be possible for other models to send messages with dev_key security, although that does assume Config Client level privledges.
>
> Also, I would suggest a small rename to keep things a bit more
> consistent:
> - if SendWithDeviceKey(), then 'MessageReceivedWithDeviceKey()
> - if DevKeyMessageRedeived(), then 'DevKeySend()
>
> but in general, I'd rather rename the original Send/MessageReceived, so that
> we end up with:
> - SendApplicationMessage + ApplicationMessageReceived
> - SendDeviceMessage + DeviceMessageReceived
> - SendControlMessage + ControlMessageReceived (but I don't see a need
> to expose this to application, at least not at the moment)
That seems reasonable. We will rework for naming consistency.
I am not sure there are *any* control messages that I would expose to any of the Apps. Certainly none of the existing set of control messages, and I worry that exposing the Control interface would be seen as an invitation for "Vendor Usage" which could be difficult to back out later.
