Claudio > -----Original Message----- > From: Claudio Takahasi [mailto:claudio.takahasi@xxxxxxxxxxxxx] > Sent: Monday, March 26, 2012 10:31 PM > To: Ganir, Chen > Cc: linux-bluetooth@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v2 0/5] Add DeviceInformationService plugin > > Hi Chen Ganir: > > On Mon, Mar 26, 2012 at 12:25 PM, <chen.ganir@xxxxxx> wrote: > > From: Chen Ganir <chen.ganir@xxxxxx> > > > > Add Device Information Service GATT Client for reading peer > > device PNP_ID and store it. > > > > This is v2 of the patch set, removing some unused code to > > eliminate compilation warnings. > > I am afraid of race conditions between HoG and DIS services. > > ATT is a "serial" protocol, however there is no guarantee that PNP ID > characteristic value will be available before creating uhid device. It > depends on the probing order AND when the commands are added in the > GAttrib queue. > Currently, DID is read during the CreateDevice (BR/EDR over SDP when device is created). In addition, when we create a device, we immediately probe the DIS plug-in which immediately starts exploring the remote DIS server, even before uhid is created. A /dev/uhid device will be opened only after a device has been created, and a user action was performed to connect the input device (org.bluez.Input.Connect will be called only after you get the DeviceCreated event, which means that all data has been read) , so we do not need to worry about such a scenario at this stage. However, I do think we need to design a mechanism for synchronizing between plug-ins, but I'm not sure it should be a generic plug-in sync mechanism, since not all plug-ins require this. > IMO, the alternatives are: > 1. move the DIS/PnP discovery to the create device "procedure" It is already done this way. CreateDevice for LE device calls the discover_primaries, and then probes all drivers. The DIS plug-in will be initialized, and read the values from the peer device, and store them in persistent storage. > 2. create plugin states and dependency before probing We do not have to protect the probing here. We do need to protect the /dev/uhid creation and configuration. A /dev/uhid will be created on probing only after the user has previously selected the remote device for connection, and this will happen only after we have a valid PNP id already stored. > 3. uuid "notification" mechanism": callback gets called when a new > value is available for a given characteristic A callback is part of the solution, and needs to be accompanied by a way to query the plugin for the value of a specific characteristic by uuid. However, this may not be easy. For example, you may have more than one battery services, with battery level characteristics (with the same UUID). How do you link between a specific battery and its value? I do not think a generic solution is the best solution here. Perhaps we need to combine this with the btd_device, and make sure btd_device is the central location for getting values and notifications. Currently we have the btd_device_get_vendor_src, btd_device_get_vendor and others which can be used to check if a value is present. Or not. The simplest solution would be checking this information periodically from the HID service plugin, or add a property_changed callback similar to the D-BUS mechanism that would have a variable sized data payload, depending on the property id we use. An example would be : typedef void (*property_changed_cb) (uint16_t property_id, uint8_t *property_data, uint16_t data_length) #define PROPERTY_CHANGED_VENDOR_ID 0x10 typedef struct vid_changed { uint16_t vid; } #define PROPERTY_CHANGED_VENDOR_ID_SRC 0x11 typedef struct vid_src_changed { uint16_t vid_src; } #define PROPERTY_CHANGED_BATTERY_LEVEL 0x12 typedef struct batt_level_changed { uint8_t namespace; uint16_t description; uint8_t value; } this will be called in conjunction with the D-BUS PropertyChanged event for each changed property, for all registered property_changed callbacks registered. What do you think ? > > Opinions? > > BR, > Claudio Thanks, Chen Ganir ��.n��������+%������w��{.n�����{��� �^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
