Re: [PATCH BlueZ] mesh: Log D-Bus method call errors

["michal.lowas-rzechonek@silvair.com" <michal.lowas-rzechonek@silvair.com>]

Hi Brian,

On 08/28, Gix, Brian wrote:
> On Tue, 2019-08-20 at 09:56 +0200, Michał Lowas-Rzechonek wrote:
> > If a system is misconfigured, mesh daemon might not have permissions to
> > call application methods.
> >
> > This patch causes mesh daemon to log such errors, instead of failing
> > silently.
>
> Some of these Replies for error checking are warranted, I think...
> Particularily when there is required information that needs to be sent
> to the Application during Provisioning, for instance.
>
> But sometimes we expect the application to be "away" for normal
> reasons (it is intended as a foreground app, for instance) where I am
> not sure we want to require the response... For instance the method
> calls in model.c that occur when a remote node has sent a message.

Yes, these calls were my primary concern here.

Note that D-Bus calls do *not* happen if the application is not attached
(node->owner is NULL).

> The Non-Reply version of send (towards the apps) was actually a design
> decision, since we don't want the *daemon* to exhast d-bus resources,
> depending on replies from Apps that are ignoring the messages we are
> sending.
>
> This could negatively impact the daemon's ability to
> interact with perhaps better behaved applications.  I think every
> reply required message persists for up to 30 seconds.

True.

Since most of the application-side methods do not return anything (and
rightly so, because "Any discrete OTA message might be lost"), the
application is free to do whatever is pleases with the payload,
including dropping it.

Still, I think that the none of the call handlers on the application
side should *ever* return errors/timeouts over D-Bus.

I'm arguing that such an application is misbehaving, so it probably
should be promptly detached. That would protect the daemon.

> I think our rule of thumb should be requiring a response when the
> daemon needs to know that the App has successfully handled critical
> information so for instance YES for:
>
> AddNodeComplete()
> JoinComplete()
> RequestProvData()

Agreed.

regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND