RE: Proposal for GATT Client Dbus API.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Vijay,


> -----Original Message-----
> From: Vijaykumar Dadmode [mailto:Vijaykumar.Dadmode@xxxxxxx]
> Sent: Monday, November 07, 2011 11:20 AM
> To: Ganir, Chen
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx
> Subject: RE: Proposal for GATT Client Dbus API.
> 
> Hi Chen,
> Could you please clarify below points:
> > > > application which requests for it?
> > > We can use the existing watchers. At the moment, watchers are
> > registered to be called when indication/notification arrives from the
> > >
> > > sever. This means that a watcher will receive ValueChanged message
> > with characteristic path and new value. We could use this >
> >interface
> > >and add the ReadRespnse and WriteResponse to it.
> >
> 
> So you mean we add the ReadResponse and WriteResponse to Watcher
> Hierarchy?  , but what if we don't register the watcher and just want
> to read/write?
> 
If this is the case, you will not get any responses to your write and read commands. I think it is ok to force the user to register a watcher, to get callbacks. I do not feel good with broadcasting such events to everyone. 

> 
> > This custom descriptor is defined as an array of bytes with specific
> uuid. It can be reflected to the user as a property, which will > be
> available if the server contains such a descriptor for the
> characteristic. This way, we can use the getproperty and setproperty,
> > and hide all the irrelevant descriptor/handle logic from the user.
> 
> The custom desc can be:
> * There can be more than one "custom" descriptor.
> * The descriptor value can have any size.
> * And more important: "Characteristic descriptor declaration
> permissions are defined by a higher layer profile or are implementation
> specific. A client shall not assume all characteristic descriptor
> declarations are readable."
> 
> Proposal 1: We can have custom specific desc calls ReadCustomDescriptor
> and WriteCustomDescriptor.
> Proposal 2: Have a property Custom, and then access the descriptor as
> Attributes.
> 
I would go with Custom property. We can even set a numeric or even add the handle, as done for serives and characteristics, something like Custom0002, Custom 001a and so on, for supporting multiple custom characteristic descriptors.




> Thanks,
> Vijay
> 
> -----Original Message-----
> From: Ganir, Chen [mailto:chen.ganir@xxxxxx]
> Sent: Sunday, November 06, 2011 1:25 PM
> To: Vijaykumar Dadmode
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx
> Subject: RE: Proposal for GATT Client Dbus API.
> 
> Vijay,
> 
> 
> > -----Original Message-----
> > From: Vijaykumar Dadmode [mailto:Vijaykumar.Dadmode@xxxxxxx]
> > Sent: Friday, November 04, 2011 2:46 PM
> > To: Ganir, Chen
> > Cc: linux-bluetooth@xxxxxxxxxxxxxxx
> > Subject: RE: Proposal for GATT Client Dbus API.
> >
> > Hi Chen,
> > Thanks for your inputs. Please find my comments inline.
> >
> > > > Queries:
> > > > 1) As the "Signals" are a broadcast, how do we direct it to the
> > > > application which requests for it?
> > > We can use the existing watchers. At the moment, watchers are
> > registered to be called when indication/notification arrives from the
> >
> > > sever. This means that a watcher will receive ValueChanged message
> > with characteristic path and new value. We could use this >
> >interface
> > >and add the ReadRespnse and WriteResponse to it.
> >
> > So you mean we add the ReadResponse and WriteResponse to Watcher
> > Hierarchy?  , but what if we don't register the watcher and just want
> > to read/write?
> >
> > >>
> > >> +		void GetCharacteristicValue( )
> > >> +
> > >> +			Read the value of the characteristic.
> > >> +			This will emit a ReadResponse signal with
> status
> > >> code.
> >
> >
> > >When will this signal/message be sent ? Do we implement a timeout
> for
> > connection timeout ? Can we set it as a parameter to this message?
> >
> >
> > Method can be called when applications want to read the value on
> > server(active read).
> >
> This I understand.
> Since this is an async function, we must have some timeout, or
> mechanism to notify the user that we're either connected and fetching
> the value, and he should be expecting a ReadResponse anytime soon, or
> that we are not currently connected, and we will try a connection
> procedure to the server, which may take some time, or may even never
> end. Now that I think of it - maybe we also need
> CancelGetCharacteristicValue function, to remove a pending read value
> request - we do not want the ReadResponse event triggered if the
> connection is established after a few days for example.
> 
> > > > +
> > > > +		void SetCharacteristicValue(variant value, string
> method,
> > > uint16 offset)
> > > > +
> > > > +			Changes the value of the specified
> characteristic as
> > > > per the offset and method. Only
> > > > +			read-write characteristic value can be changed.
> > > What is this read-write char value you are referring to here ?
> >
> > Updated.
> >
> Same as above - we need to be able to cancel a set request, and we need
> some timeout to notify the user that the DBUS client has given up and
> will not try to set the value any more.
> 
> > > +			This will emit a WriteResponse signal with status
> > > code.
> > > Again, we need some timeout, to allow clean upper level
> > implementation.
> >
> > > +
> > > +			Write the value of the characreristic, with any of
> > > these specified method:
> > > +			"WriteWithResponse", "SignedWriteWithoutResponse",
> > > +			"WriteLongCharacteristicValue", "PrepareWrite",
> > > "ExecuteWrite".
> > > +
> > > +		void  ReadDescriptor(uint16 handle)
> > > +
> > > +			Read the value of the specified characteristic
> > > descriptor matching the handle.
> > > +			This will emit a ReadDescriptorResponse signal with
> > > status code.
> > > This will not go through. Handles should not be used by the users.
> In
> > fact, all descriptors can be turned into D-BUS dictionaries, >and be
> > available as characteristic properties. This is already done today
> for
> > some descriptors. However, not all of them should be >exposed
> directly
> > to user (client characteristic config descriptor for example can be
> > exposed as properties Notify and Indicate, server >char config
> > descriptor can be exposed as property Broadcast, and so on. Format is
> > already discovered and partially used in the >existing code.
> >
> > ReadDescriptor and WriteDescriptor we have added keeping "Custom"
> > descriptors (Optional Characteristic descriptor which
> > defines profile defined descriptor UUID's.)  in mind.
> > > +		dict Descriptors [readonly]
> >
> This custom descriptor is defined as an array of bytes with specific
> uuid. It can be reflected to the user as a property, which will be
> available if the server contains such a descriptor for the
> characteristic. This way, we can use the getproperty and setproperty,
> and hide all the irrelevant descriptor/handle logic from the user.
> 
> > So a property which contains a dict (mapping handle -> uuid) for
> > *all* descriptors (of course the properties for known descriptors
> > would still exist, but this generic read/write API would be used for
> > them as well).
> >
> This is wrong. This means that you duplicate the data - once in single
> flags (notify, indicate, user_defined) and once as a set of raw
> descriptor data. Again, I believe this should be hidden from the user,
> who does not need to handle descriptors and handles.
> 
> >
> >
> > > +
> > > +		void  WriteDescriptor(uint16 handle, array{byte} value)
> > > +
> > > +			Changes the value of the specified characteristic
> > > descriptor matching the handle. Only
> > > +			read-write descriptors can be changed.
> > > +			This will emit a WriteDescriptorResponse signal with
> > > status code.
> > > +
> > >  			Possible Errors: org.bluez.Error.InvalidArguments
> > >
> > > No use for this one - see above my previous comment.
> >
> > > +Signals		PropertyChanged(string name, variant value)
> > > +
> > > +			This signal indicates a changed value of the given
> > > +			property.
> > > +
> > > When will this be used? We already have readresponse,
> writeresponse.
> > Other properties are fixed, and read on connection > >automatically.
> We
> > will not need this. We can use this if we expose Notify, Indicate,
> > Broadcast properties. Writing to these properties >will cause the
> > propertychanged event to be called, once the write has completed, if
> > that's possible.
> >
> > Yes agree with adding Notify, Indicate, Broadcast as properties.
> >
> >
> >
> > > +		ReadResponse(uint8 status, variant value)
> > > +
> > > +			Parameter is the read value and status code for
> > > GetCharacteristicValue operation.
> > >We need to add the characteristic path as well. We may issue
> multiple
> > read requests, and receive them all after a while - we need to >be
> able
> > to understand which characteristic value we received.
> >
> > Agreed.
> >
> > > +
> > > +		WriteResponse(uint8 status)
> > > +
> > > +			Parameter is the status code for
> > > SetCharacteristicValue operation.
> > >Same as above - we need the characteristic path.
> >
> > Agreed.
> >
> > > +
> > > +
> > > +		ReadDescriptorResponse(uint8 status, variant value)
> > > +
> > > +			Parameter is the read value and status code for
> > > ReadDescriptor operation.
> > > +
> > > +		WriteDescriptorResponse(uint8 status)
> > > +
> > > +			Parameter is the status code for WriteDescriptor
> > > operation.
> > > +
> > >No need for this.
> >
> > Added keeping "Custom" descriptors in mind.
> >
> See above about custom descriptors.
> 
> 
> > >  Properties 	string UUID [readonly]
> > >
> > >  			UUID128 of this characteristic.
> > > @@ -121,12 +169,22 @@ Properties 	string UUID [readonly]
> > >  			Optional field containing a friendly name for the
> > >  			Characteristic UUID.
> > >
> > > +		array{string} Properties [readonly]
> > > +
> > > +			The characteristic properties includes extended
> > > properties if defined, format defined by GATT spec.
> > > +			Possible values representing each bit field:
> > > +			{ "Broadcast", "Read", "WriteWithoutResponse",
> > > "Write", "Notify", "Indicate", "AuthenticatedSignedWrite",
> > > 			"ExtentedProperties", "ReliableWrite",
> > > "WritableAuxiliaries"}
> > > +
> > >Properties which can be changed, such as Broadcast, Indicate, Notify
> > should be standalong, Boolean read/write properties. See above >for
> > more information on why we need these.
> >
> > Agreed. I think we can have this array plus for the simpler usage we
> > can add the r/w Broadcast, Indicate, Notify properties.
> >
> Why duplicate data again ? it is very confusing to have this
> information in two or more places. Writable properties should have
> their own representation, without duplication in this list.
> 
> > > +		uint16 ClientConfiguration [readwrite]
> > > +
> > > +			Optional field containing the client configuration
> > > value on the server.
> > > +
> > > We should not expose bit fields to the users. We can abstract this
> to
> > Notify/Indicate Boolean properties.
> >
> > Agreed
> >
> > >>  		string Description [readonly]
> > >>
> > >  			Textual optional characteristic descriptor describing
> > >  			the Characteristic Value.
> > >
> > > -		struct Format [readonly]
> > > +		array{struct} Format [readonly]
> > >
> > >  			Optional Characteristic descriptor which defines the
> > >  			format of the Characteristic Value. For numeric
> > > @@ -151,6 +209,11 @@ Properties 	string UUID [readonly]
> > >  			Friendly representation of the Characteristic Value
> > >  			based on the format attribute.
> > >
> > > This also needs to be simplified. We need to take the format
> strings
> > from the spec, and apply the correct properties according to the >
> > format descriptor. Again, we should not expose raw bit fields here.
> >
> > >> +		dict Descriptors [readonly]
> > >> +
> > >> +			The value is a dictionary for all
> Characteristic
> > >> descriptor with the handle as keys and the UUID as values.The key
> is
> > > 			uint16 and the value a string for this dictionary.
> > > Can be used with WriteDescriptor and ReadDescriptor.
> > > +		      These attribute handle is only valid during the
> > >> session, and should not be stored/cached by the client.
> > >> +
> > > See above. Not needed.
> > Explained part of ReadDescriptor.
> >
> See above why we should not expose raw descriptors to the user. We have
> the properties mechanism, which is what descriptors are all about.
> Simply translate the GATT descriptors into DBUS properties.
> 
> > Thanks,
> > Vijay
> >
> >
> >
> > Member of the CSR plc group of companies. CSR plc registered in
> England
> > and Wales, registered number 4187346, registered office Churchill
> > House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ,
> United
> > Kingdom
> > More information can be found at www.csr.com. Follow CSR on Twitter
> at
> > http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
> 
> 
> Vijay, can you send an updated proposal with the changes we've
> discussed, or should I send it ?
> 
> Thanks,
> Chen Ganir.
> Texas Instruments.
> 
> 
>  To report this email as spam click
> https://www.mailcontrol.com/sr/keBbNGaYv7HTndxI!oX7UsdpzMR7Bo2KP2imJfv6
> 99sbiy929P9Qs4lEOZpe1nNVJnoe!+NpOToakVWf!6zCig== .

Thanks,
Chen Ganir
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux