[EXT] Re: [PATCH v6] Add the similar USD APIs to dbus control interface that other apps can use the functions

[Chin-Ran Lo]

Hi Jouni,
Thank you for the suggestions. I have pushed a new v7 patch
V7 1/2: It's the 1st one you sent back
V7 2/2: Please see the comment below.

Thank you
Chin-Ran

> -----Original Message-----
> From: Jouni Malinen <j at w1.fi>
> Sent: Thursday, August 1, 2024 1:02 AM
> To: Chin-Ran Lo <chin-ran.lo at nxp.com>
> Cc: hostap at lists.infradead.org; Pete Hsieh <tsung-hsien.hsieh at nxp.com>
> Subject: [EXT] Re: [PATCH v6] Add the similar USD APIs to dbus control
> interface that other apps can use the functions
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On Fri, Jul 26, 2024 at 12:38:01PM +0000, Chin-Ran Lo wrote:
> >  wpa_supplicant/dbus/dbus_new.c          | 261 +++++++++++++++
> >  wpa_supplicant/dbus/dbus_new.h          |  35 ++
> >  wpa_supplicant/dbus/dbus_new_handlers.c | 416
> > ++++++++++++++++++++++++  wpa_supplicant/dbus/dbus_new_handlers.h
> |
> > 13 +-  wpa_supplicant/dbus/dbus_new_helpers.h  |  14 +
> >  wpa_supplicant/events.c                 |   2 +-
> >  wpa_supplicant/nan_usd.c                |   9 +-
> >  wpa_supplicant/notify.c                 |  27 ++
> >  wpa_supplicant/notify.h                 |  11 +
> >  9 files changed, 785 insertions(+), 3 deletions(-)
> 
> It would be cleaner to split this into two commits: one for handling the move
> of the control interface specific code from nan_usd.c into notify.c/h and
> another one to add the dbus specific changes.
> 
> As far as the proposed dbus interface is concerned, my main concern is on how
> the commands are defined to take in a single string argument that is then
> parsed to determine the set of actual arguments. That is not really a clean
> interface for dbus. Instead, this should add the individual arguments as
> separate dbus arguments with each having their own (in most cases, non-string)
> type.

Chin-Ran> The interface has been modified in 2/2 patch. Please review

> 
> Why does this not add NANReplied event?

Chin-Ran> It's added in 2/2 patch. Please review.

> 
> Why does this leave out some of the arguments from events (like
> FSD/FSD-GAS)?
> 

Chin-Ran> They are added in 2/2 patch. Please review.


> It would be good to update doc/dbus.doxygen to cover the new extension to
> the dbus interface.

Chin-Ran> It's added in 2/2 patch. Please review.


> 
> It would be nice to get a new tests/hwsim/test_dbus.py test case for validating
> the new interface.

Chin-Ran> The functions are verified by using the Matter code now. I will also add the test code to here later.


> 
> I'm attaching patches showing the two commits after a cleanup I did when
> reviewing this. The first one should be ready to be applied as-is. The second
> one needs those comments mentioned above addressed.
> 
> --
> Jouni Malinen                                            PGP id
> EFC895FA