diff mbox

dbus: Making the peer's properties acessible separately

Message ID CAKxyoHmefA+aK+KVFMbdHrC3n60GbNuOZ51Qq-VSs4iSctijyQ@mail.gmail.com
State Accepted
Commit 3f6e50ac282bbcb4be137023316543bd232ba350
Headers show

Commit Message

Flávio Ceolin Feb. 3, 2012, 8:52 p.m. UTC
How there is the method org.freedesktop.DBus.Properties.GetAll
that returns all properties from a specific interface, makes more
sense separate the properties to be possible to get only a specific
property using the method org.freedesktop.DBus.Properties.Get.

Signed-hostap: Flavio Ceolin <flavio.ceolin@profusion.mobi>
---
 wpa_supplicant/dbus/dbus_new.c              |   32 +++-
 wpa_supplicant/dbus/dbus_new_handlers_p2p.c |  269 ++++++++++++++++++++-------
 wpa_supplicant/dbus/dbus_new_handlers_p2p.h |   36 ++++-
 3 files changed, 264 insertions(+), 73 deletions(-)

 dbus_bool_t wpas_dbus_getter_p2p_peer_ies(DBusMessageIter *iter,
 					  DBusError *error,

Comments

Jouni Malinen Feb. 3, 2012, 9:13 p.m. UTC | #1
On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:
> How there is the method org.freedesktop.DBus.Properties.GetAll
> that returns all properties from a specific interface, makes more
> sense separate the properties to be possible to get only a specific
> property using the method org.freedesktop.DBus.Properties.Get.

While this may be a good change from the design view point, I'm a bit
worried about changing this interface if it breaks existing applications
that use the interface. The P2P extensions are relatively recent, so
this could still be acceptable, but I'm not going to apply this in the
current form until I hear an ack or two from people who are using this
interface.
Reinette Chatre Feb. 3, 2012, 10:01 p.m. UTC | #2
Hi Jouni and Flávio,

On Fri, 2012-02-03 at 23:13 +0200, Jouni Malinen wrote:
> On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:
> > How there is the method org.freedesktop.DBus.Properties.GetAll
> > that returns all properties from a specific interface, makes more
> > sense separate the properties to be possible to get only a specific
> > property using the method org.freedesktop.DBus.Properties.Get.
> 
> While this may be a good change from the design view point, I'm a bit
> worried about changing this interface if it breaks existing applications
> that use the interface. The P2P extensions are relatively recent, so
> this could still be acceptable, but I'm not going to apply this in the
> current form until I hear an ack or two from people who are using this
> interface.

We are using this interface and can support this change. Since this is
an API change, could you please support this going to hostap-1? I take
the liberty to add that new tag below.

Acked-by: Reinette Chatre <reinette.chatre@intel.com>
intended-for: hostap-1

Flávio: I am just curious ... are you planning a similar change to the
P2PDeviceProperties on the P2PDevice interface?

Reinette
Flávio Ceolin Feb. 3, 2012, 11:30 p.m. UTC | #3
Hi Jouni and Reinette,

On Fri, Feb 3, 2012 at 8:01 PM, Reinette Chatre
<reinette.chatre@intel.com> wrote:
> Hi Jouni and Flávio,
>
> On Fri, 2012-02-03 at 23:13 +0200, Jouni Malinen wrote:
>> On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:
>> > How there is the method org.freedesktop.DBus.Properties.GetAll
>> > that returns all properties from a specific interface, makes more
>> > sense separate the properties to be possible to get only a specific
>> > property using the method org.freedesktop.DBus.Properties.Get.
>>
>> While this may be a good change from the design view point, I'm a bit
>> worried about changing this interface if it breaks existing applications
>> that use the interface. The P2P extensions are relatively recent, so
>> this could still be acceptable, but I'm not going to apply this in the
>> current form until I hear an ack or two from people who are using this
>> interface.
>
> We are using this interface and can support this change. Since this is
> an API change, could you please support this going to hostap-1? I take
> the liberty to add that new tag below.
>
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> intended-for: hostap-1
>
> Flávio: I am just curious ... are you planning a similar change to the
> P2PDeviceProperties on the P2PDevice interface?
>
> Reinette
>
>
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap

Thanks for the feedback folks.

Reinnet, answering your question, yes I've plan to do it,
I'm just waiting the feedback for these changes.

I think it's a good time to do it since this code is still
recent.

Regards,
Flávio Ceolin
Chinchilla, Angie V Feb. 4, 2012, 12:02 a.m. UTC | #4
On Fri, 2012-02-03 at 14:01 -0800, Chatre, Reinette wrote:
> Hi Jouni and Flávio,

> 

> On Fri, 2012-02-03 at 23:13 +0200, Jouni Malinen wrote:

> > On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:

> > > How there is the method org.freedesktop.DBus.Properties.GetAll

> > > that returns all properties from a specific interface, makes more

> > > sense separate the properties to be possible to get only a specific

> > > property using the method org.freedesktop.DBus.Properties.Get.

> >

> > While this may be a good change from the design view point, I'm a bit

> > worried about changing this interface if it breaks existing

> applications

> > that use the interface. The P2P extensions are relatively recent, so

> > this could still be acceptable, but I'm not going to apply this in

> the

> > current form until I hear an ack or two from people who are using

> this

> > interface.

> 

> We are using this interface and can support this change. Since this is

> an API change, could you please support this going to hostap-1? I take

> the liberty to add that new tag below.

> 

> Acked-by: Reinette Chatre <reinette.chatre@intel.com>

> intended-for: hostap-1

> 

> Flávio: I am just curious ... are you planning a similar change to the

> P2PDeviceProperties on the P2PDevice interface?


It would be great if we could get the P2P DBus interface cleaned up before releasing hostap-1.0. Obviously the interface becomes harder to tweak once we release. I'm all for taking these changes late, if others agree.

-Angie
Dan Williams Feb. 6, 2012, 3:40 p.m. UTC | #5
On Fri, 2012-02-03 at 21:30 -0200, Flávio Ceolin wrote:
> Hi Jouni and Reinette,
> 
> On Fri, Feb 3, 2012 at 8:01 PM, Reinette Chatre
> <reinette.chatre@intel.com> wrote:
> > Hi Jouni and Flávio,
> >
> > On Fri, 2012-02-03 at 23:13 +0200, Jouni Malinen wrote:
> >> On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:
> >> > How there is the method org.freedesktop.DBus.Properties.GetAll
> >> > that returns all properties from a specific interface, makes more
> >> > sense separate the properties to be possible to get only a specific
> >> > property using the method org.freedesktop.DBus.Properties.Get.
> >>
> >> While this may be a good change from the design view point, I'm a bit
> >> worried about changing this interface if it breaks existing applications
> >> that use the interface. The P2P extensions are relatively recent, so
> >> this could still be acceptable, but I'm not going to apply this in the
> >> current form until I hear an ack or two from people who are using this
> >> interface.
> >
> > We are using this interface and can support this change. Since this is
> > an API change, could you please support this going to hostap-1? I take
> > the liberty to add that new tag below.
> >
> > Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> > intended-for: hostap-1
> >
> > Flávio: I am just curious ... are you planning a similar change to the
> > P2PDeviceProperties on the P2PDevice interface?
> >
> > Reinette
> >
> >
> > _______________________________________________
> > HostAP mailing list
> > HostAP@lists.shmoo.com
> > http://lists.shmoo.com/mailman/listinfo/hostap
> 
> Thanks for the feedback folks.
> 
> Reinnet, answering your question, yes I've plan to do it,
> I'm just waiting the feedback for these changes.
> 
> I think it's a good time to do it since this code is still
> recent.

If you want to maintain backwards compatibility, just break the
properties out like your patch did, but *also* retain the 'Properties'
property from the old code.  Thus it wouldn't break current users, but
we'd take advantage of GetAll.

Dan
Reinette Chatre Feb. 6, 2012, 5:50 p.m. UTC | #6
Hi Dan,

On Mon, 2012-02-06 at 09:40 -0600, Dan Williams wrote:
> If you want to maintain backwards compatibility, just break the
> properties out like your patch did, but *also* retain the 'Properties'
> property from the old code.  Thus it wouldn't break current users, but
> we'd take advantage of GetAll.

Can this perhaps be avoided? There has not been an official
wpa_supplicant release with P2P so I don't think backwards compatibility
is a major issue here. I think having two places in the D-Bus API with
the same properties available will introduce a lot of confusion. In
addition to the confusion, if we now proceed with an official release
with the two spots for properties the original somehow (any ideas?)
needs to be marked as deprecated (in the first release it is
available :) ) and hopefully removed at some point, otherwise it will
need to be carried forever, which I think will be unnecessary burden and
a potential source of inconsistencies.

Reinette
Flávio Ceolin Feb. 6, 2012, 7:03 p.m. UTC | #7
Hi  Reinette

On Mon, Feb 6, 2012 at 3:50 PM, Reinette Chatre
<reinette.chatre@intel.com> wrote:
> Hi Dan,
>
> On Mon, 2012-02-06 at 09:40 -0600, Dan Williams wrote:
>> If you want to maintain backwards compatibility, just break the
>> properties out like your patch did, but *also* retain the 'Properties'
>> property from the old code.  Thus it wouldn't break current users, but
>> we'd take advantage of GetAll.
>
> Can this perhaps be avoided? There has not been an official
> wpa_supplicant release with P2P so I don't think backwards compatibility
> is a major issue here. I think having two places in the D-Bus API with
> the same properties available will introduce a lot of confusion. In
> addition to the confusion, if we now proceed with an official release
> with the two spots for properties the original somehow (any ideas?)
> needs to be marked as deprecated (in the first release it is
> available :) ) and hopefully removed at some point, otherwise it will
> need to be carried forever, which I think will be unnecessary burden and
> a potential source of inconsistencies.
>
> Reinette
>
>

That's exactly what I think. As we don't have an official release, it seems to
be an error this kind of inconsistency.

BR,
Flávio Ceolin
Jouni Malinen Feb. 11, 2012, 9:43 a.m. UTC | #8
On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:
> How there is the method org.freedesktop.DBus.Properties.GetAll
> that returns all properties from a specific interface, makes more
> sense separate the properties to be possible to get only a specific
> property using the method org.freedesktop.DBus.Properties.Get.

Thanks! Applied.

> diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
>  static const struct wpa_dbus_property_desc wpas_dbus_p2p_peer_properties[] = {
> +	{"config_method", WPAS_DBUS_NEW_IFACE_P2P_PEER, "n",
> +	{"level", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
> +	{"SecondaryDeviceTypes", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",

There were incorrect D-Bus types in the introspect information. I tried
to correct these (and one type in the handler functions, too). Please
verify that the end result matches with what you would like to see
there.
Dan Williams Feb. 11, 2012, 11:29 p.m. UTC | #9
On Sat, 2012-02-11 at 11:43 +0200, Jouni Malinen wrote:
> On Fri, Feb 03, 2012 at 06:52:18PM -0200, Flávio Ceolin wrote:
> > How there is the method org.freedesktop.DBus.Properties.GetAll
> > that returns all properties from a specific interface, makes more
> > sense separate the properties to be possible to get only a specific
> > property using the method org.freedesktop.DBus.Properties.Get.
> 
> Thanks! Applied.
> 
> > diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
> >  static const struct wpa_dbus_property_desc wpas_dbus_p2p_peer_properties[] = {
> > +	{"config_method", WPAS_DBUS_NEW_IFACE_P2P_PEER, "n",
> > +	{"level", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
> > +	{"SecondaryDeviceTypes", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
> 
> There were incorrect D-Bus types in the introspect information. I tried
> to correct these (and one type in the handler functions, too). Please
> verify that the end result matches with what you would like to see
> there.

Hmm, using 'n' is pretty weird here since that's a 16-bit signed
integer.  I wonder why it wasn't just 'u' (32-bit unsigned).  The 'au'
is "array of uint32" which should be fine, but something else odd here
is that the P2P properties are mixed between StudlyCaps and
underscore_separated formats.  For consistency they should probably be
using one or the other, preferably StudlyCaps since that's the dbus
convention.

Dan
Jouni Malinen Feb. 12, 2012, 10:28 a.m. UTC | #10
On Sat, Feb 11, 2012 at 05:29:52PM -0600, Dan Williams wrote:
> > > +	{"config_method", WPAS_DBUS_NEW_IFACE_P2P_PEER, "n",
> > > +	{"level", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
> > > +	{"SecondaryDeviceTypes", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",

> Hmm, using 'n' is pretty weird here since that's a 16-bit signed
> integer.  I wonder why it wasn't just 'u' (32-bit unsigned).

I replaced that with "q" to match with DBUS_TYPE_UINT16.

> The 'au' is "array of uint32" which should be fine,

Well.. level is actually a signed integer, so "au" was quite odd for it
(I replaced this with "i"). As far as SecondaryDeviceTypes are
concerned, array of 32-bit unsigned integers sounds a bit odd. The
device type is an 8-octet field (which can also be presented as a string
like 1-0050F204-1) and this field would be an array of those. For now, I
replaced that with "ay", but I'm not sure whether that really is the
best approach.

> but something else odd here
> is that the P2P properties are mixed between StudlyCaps and
> underscore_separated formats.  For consistency they should probably be
> using one or the other, preferably StudlyCaps since that's the dbus
> convention.

I was tempted to change these, but then noticed that these came from the
previously used dictionary and left the strings unchanged.
Reinette Chatre Feb. 14, 2012, 12:34 a.m. UTC | #11
On Sun, 2012-02-12 at 12:28 +0200, Jouni Malinen wrote:
> On Sat, Feb 11, 2012 at 05:29:52PM -0600, Dan Williams wrote:
> > > > +	{"config_method", WPAS_DBUS_NEW_IFACE_P2P_PEER, "n",
> > > > +	{"level", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
> > > > +	{"SecondaryDeviceTypes", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
> 
> > Hmm, using 'n' is pretty weird here since that's a 16-bit signed
> > integer.  I wonder why it wasn't just 'u' (32-bit unsigned).
> 
> I replaced that with "q" to match with DBUS_TYPE_UINT16.
> 
> > The 'au' is "array of uint32" which should be fine,
> 
> Well.. level is actually a signed integer, so "au" was quite odd for it
> (I replaced this with "i"). As far as SecondaryDeviceTypes are
> concerned, array of 32-bit unsigned integers sounds a bit odd. The
> device type is an 8-octet field (which can also be presented as a string
> like 1-0050F204-1) and this field would be an array of those. For now, I
> replaced that with "ay", but I'm not sure whether that really is the
> best approach.

When the patch came in I wrongly assumed that the types will remain as
before, just the interface changed ... Thanks to Jouni for catching
these. From what I can tell the vendor extensions property type is still
different from before. Could this not perhaps remain as bytes?

Reinette
Chinchilla, Angie V Feb. 15, 2012, 7:26 p.m. UTC | #12
On Fri, 2012-02-03 at 15:30 -0800, Flávio Ceolin wrote:
> On Fri, Feb 3, 2012 at 8:01 PM, Reinette Chatre
> <reinette.chatre@intel.com> wrote:
> > Hi Jouni and Flávio,
> >
> > Flávio: I am just curious ... are you planning a similar change to the
> > P2PDeviceProperties on the P2PDevice interface?
> >
> Thanks for the feedback folks.
> 
> Reinnet, answering your question, yes I've plan to do it,
> I'm just waiting the feedback for these changes.

Hi Flávio,

Just checking in. Do you happen to have a patch in the works for
P2PDeviceProperties on the P2PDevice interface? 

Thanks,
Angie
Jouni Malinen Feb. 15, 2012, 8:05 p.m. UTC | #13
On Mon, Feb 13, 2012 at 04:34:18PM -0800, Reinette Chatre wrote:
> When the patch came in I wrongly assumed that the types will remain as
> before, just the interface changed ... Thanks to Jouni for catching
> these. From what I can tell the vendor extensions property type is still
> different from before. Could this not perhaps remain as bytes?

The problem there was that the previous mechanism used a dictionary and
that does not have these types defined as clearly in introspection
information.. Otherwise it would have been trivial for me to enforce
them to remain same - now I did my best, but I'm not very familiar with
D-Bus..

In other words, yes, please take a closer look at the types that are in
hostap.git now and propose a patch to change them if anything needs
modification.
Flávio Ceolin Feb. 17, 2012, 7:52 p.m. UTC | #14
Hi Angie,

On Wed, Feb 15, 2012 at 6:05 PM, Jouni Malinen <j@w1.fi> wrote:
> On Mon, Feb 13, 2012 at 04:34:18PM -0800, Reinette Chatre wrote:
>> When the patch came in I wrongly assumed that the types will remain as
>> before, just the interface changed ... Thanks to Jouni for catching
>> these. From what I can tell the vendor extensions property type is still
>> different from before. Could this not perhaps remain as bytes?
>
> The problem there was that the previous mechanism used a dictionary and
> that does not have these types defined as clearly in introspection
> information.. Otherwise it would have been trivial for me to enforce
> them to remain same - now I did my best, but I'm not very familiar with
> D-Bus..
>
> In other words, yes, please take a closer look at the types that are in
> hostap.git now and propose a patch to change them if anything needs
> modification.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap

I'm on a holiday, but the patch is pretty close to be finished,
sorry for the late. This last week I was in a hurry.

Regards,
Flavio Ceolin
Flávio Ceolin Feb. 17, 2012, 8:09 p.m. UTC | #15
Hi Angie and Jouni

2012/2/17 Flávio Ceolin <flavio.ceolin@profusion.mobi>:
> Hi Angie,
>
> On Wed, Feb 15, 2012 at 6:05 PM, Jouni Malinen <j@w1.fi> wrote:
>> On Mon, Feb 13, 2012 at 04:34:18PM -0800, Reinette Chatre wrote:
>>> When the patch came in I wrongly assumed that the types will remain as
>>> before, just the interface changed ... Thanks to Jouni for catching
>>> these. From what I can tell the vendor extensions property type is still
>>> different from before. Could this not perhaps remain as bytes?
>>
>> The problem there was that the previous mechanism used a dictionary and
>> that does not have these types defined as clearly in introspection
>> information.. Otherwise it would have been trivial for me to enforce
>> them to remain same - now I did my best, but I'm not very familiar with
>> D-Bus..
>>
>> In other words, yes, please take a closer look at the types that are in
>> hostap.git now and propose a patch to change them if anything needs
>> modification.
>>
>> --
>> Jouni Malinen                                            PGP id EFC895FA
>> _______________________________________________
>> HostAP mailing list
>> HostAP@lists.shmoo.com
>> http://lists.shmoo.com/mailman/listinfo/hostap
>
> I'm on a holiday, but the patch is pretty close to be finished,
> sorry for the late. This last week I was in a hurry.
>
> Regards,
> Flavio Ceolin

Just one more question, do you want keep compatibility
with the current P2PDevice properties interface ?

Regards,
Flavio Ceolin
Chinchilla, Angie V Feb. 22, 2012, 7:27 p.m. UTC | #16
Hi Flavio,

On Fri, 2012-02-17 at 12:09 -0800, Flávio Ceolin wrote:
> 2012/2/17 Flávio Ceolin <flavio.ceolin@profusion.mobi>:
> > Hi Angie,
> >
> > On Wed, Feb 15, 2012 at 6:05 PM, Jouni Malinen <j@w1.fi> wrote:
> >> On Mon, Feb 13, 2012 at 04:34:18PM -0800, Reinette Chatre wrote:
> >>> When the patch came in I wrongly assumed that the types will remain as
> >>> before, just the interface changed ... Thanks to Jouni for catching
> >>> these. From what I can tell the vendor extensions property type is still
> >>> different from before. Could this not perhaps remain as bytes?
> >>
> >> The problem there was that the previous mechanism used a dictionary and
> >> that does not have these types defined as clearly in introspection
> >> information.. Otherwise it would have been trivial for me to enforce
> >> them to remain same - now I did my best, but I'm not very familiar with
> >> D-Bus..
> >>
> >> In other words, yes, please take a closer look at the types that are in
> >> hostap.git now and propose a patch to change them if anything needs
> >> modification.
> >
> > I'm on a holiday, but the patch is pretty close to be finished,
> > sorry for the late. This last week I was in a hurry.
> >
> > Regards,
> > Flavio Ceolin
> 
> Just one more question, do you want keep compatibility
> with the current P2PDevice properties interface ?

Thanks for working on this patch. What do you mean by compatibility with
the current P2PDevice properties interface? 

If you're asking about keeping the function that returns all p2p device
properties at once, no, that should go away with your patch (the GetAll
replaces this).

If you're asking about keeping the individual types of the different
properties the same, then yes, I would expect those to remain the same.

-Angie
Reinette Chatre Feb. 22, 2012, 8:59 p.m. UTC | #17
This series implements the promised changes to return P2P peer property
values with the type they were returned before the recent patch to move
them to the org.freedesktop.DBus.Properties interface.

In addition to the core patch making the type changes, this series includes
a new utility function make this work easier and a fix required if
anybody wants to use "GetAll".

Please include all of this in hostap-1.0

Jayant Sane (1):
  dbus: utility to create dbus message from wpabuf array

Reinette Chatre (2):
  dbus: revert changes to some peer properties
  dbus: return NULL data for peer IEs

 wpa_supplicant/dbus/dbus_new.c              |    4 +-
 wpa_supplicant/dbus/dbus_new_handlers.c     |   67 +++++++++++++++++
 wpa_supplicant/dbus/dbus_new_handlers.h     |    6 ++
 wpa_supplicant/dbus/dbus_new_handlers_p2p.c |  104 ++++++++++++++++++++-------
 4 files changed, 154 insertions(+), 27 deletions(-)
Jouni Malinen Feb. 25, 2012, 9:12 a.m. UTC | #18
On Wed, Feb 22, 2012 at 12:59:04PM -0800, Reinette Chatre wrote:
> This series implements the promised changes to return P2P peer property
> values with the type they were returned before the recent patch to move
> them to the org.freedesktop.DBus.Properties interface.
> 
> In addition to the core patch making the type changes, this series includes
> a new utility function make this work easier and a fix required if
> anybody wants to use "GetAll".

Thanks! All three applied.
diff mbox

Patch

diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
index 726b077..bd1c6f6 100644
--- a/wpa_supplicant/dbus/dbus_new.c
+++ b/wpa_supplicant/dbus/dbus_new.c
@@ -2897,8 +2897,36 @@  int wpas_dbus_unregister_interface(struct
wpa_supplicant *wpa_s)
 #ifdef CONFIG_P2P

 static const struct wpa_dbus_property_desc wpas_dbus_p2p_peer_properties[] = {
-	{ "Properties", WPAS_DBUS_NEW_IFACE_P2P_PEER, "a{sv}",
-	  wpas_dbus_getter_p2p_peer_properties,
+	{"DeviceName", WPAS_DBUS_NEW_IFACE_P2P_PEER, "s",
+	  wpas_dbus_getter_p2p_peer_device_name,
+	  NULL
+	},
+	{"PrimaryDeviceType", WPAS_DBUS_NEW_IFACE_P2P_PEER, "ay",
+	  wpas_dbus_getter_p2p_peer_primary_device_type,
+	  NULL
+	},
+	{"config_method", WPAS_DBUS_NEW_IFACE_P2P_PEER, "n",
+	  wpas_dbus_getter_p2p_peer_config_method,
+	  NULL
+	},
+	{"level", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
+	  wpas_dbus_getter_p2p_peer_level,
+	  NULL
+	},
+	{"devicecapability", WPAS_DBUS_NEW_IFACE_P2P_PEER, "y",
+	  wpas_dbus_getter_p2p_peer_device_capability,
+	  NULL
+	},
+	{"groupcapability", WPAS_DBUS_NEW_IFACE_P2P_PEER, "y",
+	  wpas_dbus_getter_p2p_peer_group_capability,
+	  NULL
+	},
+	{"SecondaryDeviceTypes", WPAS_DBUS_NEW_IFACE_P2P_PEER, "au",
+	  wpas_dbus_getter_p2p_peer_secondary_device_types,
+	  NULL
+	},
+	{"VendorExtension", WPAS_DBUS_NEW_IFACE_P2P_PEER, "as",
+	  wpas_dbus_getter_p2p_peer_vendor_extension,
 	  NULL
 	},
 	{ "IEs", WPAS_DBUS_NEW_IFACE_P2P_PEER, "ay",
diff --git a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
index 0f6914c..547acbf 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
@@ -1172,14 +1172,13 @@  dbus_bool_t
wpas_dbus_getter_p2p_peergo(DBusMessageIter *iter,
  * Peer object properties accessor methods
  */

-dbus_bool_t wpas_dbus_getter_p2p_peer_properties(DBusMessageIter *iter,
-	DBusError *error, void *user_data)
+dbus_bool_t wpas_dbus_getter_p2p_peer_device_name(DBusMessageIter *iter,
+						  DBusError *error,
+						  void *user_data)
 {
 	struct peer_handler_args *peer_args = user_data;
-	DBusMessageIter variant_iter, dict_iter;
 	const struct p2p_peer_info *info = NULL;
-	const struct wpabuf *vendor_extension[P2P_MAX_WPS_VENDOR_EXT];
-	int i, num;
+	char *tmp;

 	if (!wpa_dbus_p2p_check_enabled(peer_args->wpa_s, NULL, NULL, error))
 		return FALSE;
@@ -1192,62 +1191,203 @@  dbus_bool_t
wpas_dbus_getter_p2p_peer_properties(DBusMessageIter *iter,
 		return FALSE;
 	}

-	if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
-					      "a{sv}", &variant_iter) ||
-	    !wpa_dbus_dict_open_write(&variant_iter, &dict_iter))
-		goto err_no_mem;
+	tmp = os_strdup(info->device_name);
+	if (!tmp) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}

-	/* Fill out the dictionary */
-	if (!wpa_dbus_dict_append_string(&dict_iter, "DeviceName",
-					 info->device_name))
-		goto err_no_mem;
-	if (!wpa_dbus_dict_append_byte_array(&dict_iter, "PrimaryDeviceType",
-					     (char *)info->pri_dev_type,
-					     WPS_DEV_TYPE_LEN))
-		goto err_no_mem;
-	if (!wpa_dbus_dict_append_uint16(&dict_iter, "config_method",
-					 info->config_methods))
-		goto err_no_mem;
-	if (!wpa_dbus_dict_append_int32(&dict_iter, "level",
-					info->level))
-		goto err_no_mem;
-	if (!wpa_dbus_dict_append_byte(&dict_iter, "devicecapability",
-				       info->dev_capab))
-		goto err_no_mem;
-	if (!wpa_dbus_dict_append_byte(&dict_iter, "groupcapability",
-				       info->group_capab))
-		goto err_no_mem;
+
+	if (!wpas_dbus_simple_property_getter(iter, DBUS_TYPE_STRING,
+					      &tmp,
+					      error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		os_free(tmp);
+		return FALSE;
+	}
+
+	os_free(tmp);
+	return TRUE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_primary_device_type(DBusMessageIter
*iter,
+                                                          DBusError *error,
+                                                          void *user_data)
+{
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
+	}
+
+	if (!wpas_dbus_simple_array_property_getter(iter, DBUS_TYPE_BYTE,
+						     (char *)info->pri_dev_type,
+						    WPS_DEV_TYPE_LEN, error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_config_method(DBusMessageIter *iter,
+                                                    DBusError *error,
+                                                    void *user_data)
+{
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
+	}
+
+	if (!wpas_dbus_simple_property_getter(iter, DBUS_TYPE_UINT16,
+					      &info->config_methods,
+					      error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_level(DBusMessageIter *iter,
+                                            DBusError *error,
+                                            void *user_data)
+{
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
+	}
+
+	if (!wpas_dbus_simple_property_getter(iter, DBUS_TYPE_INT32,
+					     &info->level,
+					     error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_device_capability(DBusMessageIter *iter,
+                                                        DBusError *error,
+                                                        void *user_data)
+{
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
+	}
+
+	if (!wpas_dbus_simple_property_getter(iter, DBUS_TYPE_BYTE,
+					     &info->dev_capab,
+					     error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_group_capability(DBusMessageIter *iter,
+                                                        DBusError *error,
+                                                        void *user_data)
+{
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
+	}
+
+	if (!wpas_dbus_simple_property_getter(iter, DBUS_TYPE_BYTE,
+					     &info->group_capab,
+					     error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_secondary_device_types(
+                                                       DBusMessageIter *iter,
+                                                       DBusError *error,
+                                                       void *user_data)
+{
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
+	}

 	if (info->wps_sec_dev_type_list_len) {
 		const u8 *sec_dev_type_list = info->wps_sec_dev_type_list;
-		int num_sec_dev_types =
-			info->wps_sec_dev_type_list_len / WPS_DEV_TYPE_LEN;
-		DBusMessageIter iter_secdev_dict_entry, iter_secdev_dict_val,
-				iter_secdev_dict_array;
-
-		if (num_sec_dev_types) {
-			if (!wpa_dbus_dict_begin_array(&dict_iter,
-						"SecondaryDeviceTypes",
-						DBUS_TYPE_ARRAY_AS_STRING
-						DBUS_TYPE_BYTE_AS_STRING,
-						&iter_secdev_dict_entry,
-						&iter_secdev_dict_val,
-						&iter_secdev_dict_array))
-				goto err_no_mem;
-			for (i = 0; i < num_sec_dev_types; i++) {
-				wpa_dbus_dict_bin_array_add_element(
-						&iter_secdev_dict_array,
-						sec_dev_type_list,
-						WPS_DEV_TYPE_LEN);
-				sec_dev_type_list += WPS_DEV_TYPE_LEN;
-			}
+		int num_sec_dev_types = info->wps_sec_dev_type_list_len;

-			if (!wpa_dbus_dict_end_array(&dict_iter,
-						&iter_secdev_dict_entry,
-						&iter_secdev_dict_val,
-						&iter_secdev_dict_array))
-				goto err_no_mem;
-		}
+		if (!wpas_dbus_simple_array_property_getter(iter,
+							    DBUS_TYPE_BYTE,
+							    sec_dev_type_list,
+							    num_sec_dev_types,
+							    error))
+			goto err_no_mem;
+		else
+			return TRUE;
+	}
+
+	if (!wpas_dbus_simple_array_property_getter(iter,
+						    DBUS_TYPE_BYTE,
+						    NULL,
+						    0,
+						    error))
+		goto err_no_mem;
+
+	return TRUE;
+
+err_no_mem:
+	dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+	return FALSE;
+}
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_vendor_extension(DBusMessageIter *iter,
+						       DBusError *error,
+						       void *user_data)
+{
+	const struct wpabuf *vendor_extension[P2P_MAX_WPS_VENDOR_EXT];
+	int i, num;
+
+	struct peer_handler_args *peer_args = user_data;
+	const struct p2p_peer_info *info = NULL;
+
+	info = p2p_get_peer_found(peer_args->wpa_s->global->p2p,
+				  peer_args->p2p_device_addr, 0);
+	if (info == NULL) {
+		dbus_set_error(error, DBUS_ERROR_FAILED, "failed to find peer");
+		return FALSE;
 	}

 	/* Add WPS vendor extensions attribute */
@@ -1258,22 +1398,15 @@  dbus_bool_t
wpas_dbus_getter_p2p_peer_properties(DBusMessageIter *iter,
 		num++;
 	}

-	if (!wpa_dbus_dict_append_wpabuf_array(&dict_iter, "VendorExtension",
-					       vendor_extension, num))
-		goto err_no_mem;
-
-	if (!wpa_dbus_dict_close_write(&variant_iter, &dict_iter) ||
-	    !dbus_message_iter_close_container(iter, &variant_iter))
-		goto err_no_mem;
+	if (!wpas_dbus_simple_array_property_getter(iter, DBUS_TYPE_STRING,
+						    vendor_extension, num, error)) {
+		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
+		return FALSE;
+	}

 	return TRUE;
-
-err_no_mem:
-	dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory");
-	return FALSE;
 }

-
 dbus_bool_t wpas_dbus_getter_p2p_peer_ies(DBusMessageIter *iter,
 					  DBusError *error, void *user_data)
 {
diff --git a/wpa_supplicant/dbus/dbus_new_handlers_p2p.h
b/wpa_supplicant/dbus/dbus_new_handlers_p2p.h
index 206e904..2f05154 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers_p2p.h
+++ b/wpa_supplicant/dbus/dbus_new_handlers_p2p.h
@@ -118,9 +118,39 @@  dbus_bool_t
wpas_dbus_getter_p2p_peergo(DBusMessageIter *iter,
 /*
  * P2P Peer properties.
  */
-dbus_bool_t wpas_dbus_getter_p2p_peer_properties(DBusMessageIter *iter,
-						 DBusError *error,
-						 void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_device_name(DBusMessageIter *iter,
+                                                  DBusError *error,
+                                                  void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_primary_device_type(DBusMessageIter
*iter,
+                                                          DBusError *error,
+                                                          void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_config_method(DBusMessageIter *iter,
+                                                    DBusError *error,
+                                                    void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_level(DBusMessageIter *iter,
+                                            DBusError *error,
+                                            void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_device_capability(DBusMessageIter *iter,
+                                                        DBusError *error,
+                                                        void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_group_capability(DBusMessageIter *iter,
+                                                       DBusError *error,
+                                                       void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_secondary_device_types(
+                                                       DBusMessageIter *iter,
+                                                       DBusError *error,
+                                                       void *user_data);
+
+dbus_bool_t wpas_dbus_getter_p2p_peer_vendor_extension(DBusMessageIter *iter,
+                                                       DBusError *error,
+                                                       void *user_data);