Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Sent: Tuesday, January 28, 2025 11:20 PM
> To: Amisha Jain (QUIC) <quic_amisjain@xxxxxxxxxxx>
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Mohammed Sameer Mulla (QUIC)
> <quic_mohamull@xxxxxxxxxxx>; Harish Bandi (QUIC)
> <quic_hbandi@xxxxxxxxxxx>; Anubhav Gupta (QUIC)
> <quic_anubhavg@xxxxxxxxxxx>
> Subject: Re: [PATCH v1] obex: Add supported features tag in MAP Client
> Connect Request
>
> Hi Amisha,
>
> On Tue, Jan 28, 2025 at 12:43 PM Amisha Jain <quic_amisjain@xxxxxxxxxxx>
> wrote:
> >
> > This change is required for below PTS testcase -
> >
> > 1. MAP/MCE/MFB/BV-06-C
> > Verify that the MCE sends its MapSupportedFeatures in the OBEX Connect
> > request if the MSE declares support for the feature
> > MapSupportedFeatures in Connect Request in its SDP record.
> >
> > If Server's SDP record contains the field 'MapSupportedFeatures in
> > Connect Request' as supported then include the supported features tag
> > in obex connect request.
>
> Can you include the btmon output with and without?
>
btmon output does not captures obex packet so adding snoop packet -
Obex Connect request with this change -
OBEX
Role: Central
Address: 0
Packet Status: Final Packet
Opcode: Connect
Length: 35
OBEX Version Number: 0x10
flags
Maximum Packet Length: 1800
Target
Header Encoding: Byte Sequence
Header ID: Target
Length: 19
Target: MAS
Application Parameters
Header Encoding: Byte Sequence
Header ID: Application Parameters
Length: 9
Parameter
Tag: Map Supported Features
Length: 4
Value
: Messages-Listing Format Version 1.1
: Extended Event Report 1.1
: Instance Information Feature
: Delete Feature
: Uploading Feature
: Browsing Feature
: Notification Feature
: Notification Registration Feature
Obex Connect request without this change -
OBEX
Role: Central
Address: 0
Packet Status: Final Packet
Opcode: Connect
Length: 26
OBEX Version Number: 0x10
flags
Maximum Packet Length: 1800
Target
Header Encoding: Byte Sequence
Header ID: Target
Length: 19
Target: MAS
> > ---
> > obexd/client/map.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/obexd/client/map.c b/obexd/client/map.c index
> > b8820335b..2fd0f74ec 100644
> > --- a/obexd/client/map.c
> > +++ b/obexd/client/map.c
> > @@ -39,6 +39,7 @@
> > #define OBEX_MAS_UUID \
> >
> "\xBB\x58\x2B\x40\x42\x0C\x11\xDB\xB0\xDE\x08\x00\x20\x0C\x9A\x66"
> > #define OBEX_MAS_UUID_LEN 16
> > +#define SUPPORTED_FEATURES_TAG 0x29
> >
> > #define MAP_INTERFACE "org.bluez.obex.MessageAccess1"
> > #define MAP_MSG_INTERFACE "org.bluez.obex.Message1"
> > @@ -2179,6 +2180,23 @@ static void parse_service_record(struct
> map_data *map)
> > map->supported_features = 0x0000001f; }
> >
> > +static void *map_supported_features(struct obc_session *session) {
> > + const void *data;
> > +
> > + /* Supported Feature Bits */
> > + data = obc_session_get_attribute(session,
> > + SDP_ATTR_MAP_SUPPORTED_FEATURES);
> > + if (!data)
> > + return NULL;
> > +
> > + if(*(uint32_t *)data & 0x00080000)
> > + return g_obex_apparam_set_uint32(NULL,
> SUPPORTED_FEATURES_TAG,
> > + 0x0000027f);
>
> Don't think it is safe to check the data like above, we don't know if field
> returned is really 32 bits, perhaps it would be a good idea to introduce
> something like obc_session_get_attribute_le32 that would ensure the value is
> really 32 bits and also check its little/big endian in the process.
>
As per the BT Spec, 32 bits field is reserved for 'MapSupportedFeatures' attribute in SDP record. So, it will be always 32 bits. Each bit corresponds to each feature. If any feature is not supported then that bit will be zero.
> > +
> > + return NULL;
> > +}
> > +
> > static int map_probe(struct obc_session *session) {
> > struct map_data *map;
> > @@ -2224,6 +2242,7 @@ static struct obc_driver map = {
> > .uuid = MAS_UUID,
> > .target = OBEX_MAS_UUID,
> > .target_len = OBEX_MAS_UUID_LEN,
> > + .supported_features = map_supported_features,
> > .probe = map_probe,
> > .remove = map_remove
> > };
> > --
> > 2.34.1
> >
> >
>
>
> --
> Luiz Augusto von Dentz
Thanks,
Amisha
