2013-02-12 11:21:37

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: [RFC] bloodpressure: Add Blood Pressure API

Change-Id: I7165ecee0186dfdb51cd01838f28768d94ff12b9
---
doc/bloodpressure-api.txt | 138 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
create mode 100644 doc/bloodpressure-api.txt

diff --git a/doc/bloodpressure-api.txt b/doc/bloodpressure-api.txt
new file mode 100644
index 0000000..2acc660
--- /dev/null
+++ b/doc/bloodpressure-api.txt
@@ -0,0 +1,138 @@
+Blood Pressure API description
+******************************
+
+
+Blood Pressure Manager hierarchy
+================================
+
+Serviceorg.bluez
+Interface org.bluez.BloodPressureManager1
+Object path [variable prefix]/{hci0,hci1,...}
+
+Methods RegisterWatcher(object agent)
+
+ Registers a watcher to monitor blood pressure
+ measurements. This agent will be notified about
+ final pressure measurements.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+
+ UnregisterWatcher(object agent)
+
+ Unregisters a watcher.
+
+ EnableIntermediateMeasurement(object agent)
+
+ Enables intermediate measurement notifications
+ for this agent. Intermediate measurements will
+ be enabled only for devices which support it.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+
+ DisableIntermediateMeasurement(object agent)
+
+ Disables intermediate measurement notifications
+ for this agent. It will disable notifications in
+ devices when the last agent removes the
+ watcher for intermediate measurements.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.NotFound
+
+Blood Pressure Profile hierarchy
+================================
+
+Service org.bluez
+Interface org.bluez.BloodPressure1
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Properties boolean Intermediate [readonly]
+
+ True if the device supports intermediate
+ measurement notifications.
+
+Blood Pressure Watcher hierarchy
+================================
+
+Service unique name
+Interface org.bluez.BloodPressureWatcher1
+Object path freely definable
+
+Methods void MeasurementReceived(object device, dict measurement)
+
+ This callback is called whenever a blood pressure
+ measurement is received from the blood pressure device.
+
+ Measurement:
+
+ string Measurement:
+
+ Posible values: "final" or
+ "intermediate"
+
+ uint16 CurrentCuffPressure:
+
+ Current cuff pressure value is only
+ present in intermediate measurement.
+
+ uint16 Systolic:
+
+ Systolic Blood Pressure value is only
+ present in final measurement.
+
+ uint16 Diastolic:
+
+ Diastolic Blood Pressure value is only
+ present in final final measurement.
+
+ uint16 Mean:
+
+ Mean Arterial Pressure value is only
+ present in final measurement.
+
+ string Unit:
+
+ The unit of blood pressure measurement.
+
+ Possible values: "mmHg" or "kPa".
+
+ uint64 Time (optional):
+
+ Time of measurement expresed in seconds
+ from epoch.
+
+ uint16 PulseRate (optional):
+
+ Pulse rate value.
+
+ uint8 UserID (optional):
+
+ User ID if the device supports
+ multiusers measurements.
+
+ boolean BodyMovement (optional)
+
+ True if body movement during
+ measurement has been detected.
+
+ boolean CuffTooLoose (optional)
+
+ True if too loose cuff during
+ measurement has been detected.
+
+ boolean IrregularPulse (optional)
+
+ True if irrgular pulse during
+ measurement has been detected.
+
+ uint8 PulseRateNotInRange (optional)
+
+ Possible values:
+
+ 0 - pulse rate exceeds upper limit
+ 1 - pulse rate is less then lower limit
+
+ boolean MeasurementPosition (optional)
+
+ True if improper measurement position
+ has been detected.
--
1.8.0.209.gf3828dc



2013-02-28 14:18:47

by Johan Hedberg

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldek,

On Thu, Feb 28, 2013, Rymarkiewicz Waldemar wrote:
> >>+Methods RegisterWatcher(object agent)
> >>+
> >>+ Registers a watcher to monitor blood pressure
> >>+ measurements. This agent will be notified about
> >>+ final pressure measurements.
> >>+
> >I'm a bit confused, what's the difference between "watcher" and "agent"?
> >If they're the same thing just pick one term and stick to it.
>
> Both mean the same, I guess. I've followed the description of
> existing profiles thermometer, cscp, hrp.

You might want to fix those in a separate patch set then. Just because
something is in the source tree doesn't mean that it's worth copying
into new code/documentation.

> Methods RegisterWatcher(object watcher)
>
> Registers a watcher to monitor blood pressure
> measurements. This watcher will be notified about
> final pressure measurements.
>
> Is this ok?

Yes, that looks better.

> >>+Blood Pressure Profile hierarchy
> >>+================================
> >>+
> >>+Service org.bluez
> >>+Interface org.bluez.BloodPressure1
> >>+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> >>+
> >>+Properties boolean Intermediate [readonly]
> >>+
> >>+ True if the device supports intermediate
> >>+ measurement notifications.
>
>
> >What's the use of this property (and the interface)? I don't see how it
> >relates to any other APIs in this specification.
>
> The device may support notifications of intermediate cuff pressure.
> That means during measurement it notifies the Collector what is
> current pressure in cuff (my device sends 5 notifications per sec)
> until it is ready to send final measurements (diastolic, systolic
> and MAP pressure).
>
> Based on this property an app can decide if it wants to
> EnableIntermediateMeasurement (CCC of right characteristic in
> configured then) or not.

Couldn't you just return a "NotSupported" error to
EnableIntermediateMeasurement?

> Similarly as in thermometer api.

I suspect there are several messed up LE APIs and implementations in the
source tree, so I'd say it's highly advisable to not just blindly copy
from the existing code but also use some common sense and be critical of
what you see.

Johan

2013-02-28 14:00:00

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Johan,

>
> Please provide a proper commit message.

Sure. Change ID will be removed in v2.

>> +Methods RegisterWatcher(object agent)
>> +
>> + Registers a watcher to monitor blood pressure
>> + measurements. This agent will be notified about
>> + final pressure measurements.
>> +
> I'm a bit confused, what's the difference between "watcher" and "agent"?
> If they're the same thing just pick one term and stick to it.

Both mean the same, I guess. I've followed the description of existing
profiles thermometer, cscp, hrp.

All of them use 'agent' term for parameter and 'watcher' in description.
I will go for watcher it seems to be more relevant.

Methods RegisterWatcher(object watcher)

Registers a watcher to monitor blood pressure
measurements. This watcher will be notified about
final pressure measurements.

Is this ok?

>
>> +Blood Pressure Profile hierarchy
>> +================================
>> +
>> +Service org.bluez
>> +Interface org.bluez.BloodPressure1
>> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>> +
>> +Properties boolean Intermediate [readonly]
>> +
>> + True if the device supports intermediate
>> + measurement notifications.


> What's the use of this property (and the interface)? I don't see how it
> relates to any other APIs in this specification.

The device may support notifications of intermediate cuff pressure. That
means during measurement it notifies the Collector what is current
pressure in cuff (my device sends 5 notifications per sec) until it is
ready to send final measurements (diastolic, systolic and MAP pressure).

Based on this property an app can decide if it wants to
EnableIntermediateMeasurement (CCC of right characteristic in configured
then) or not.

Similarly as in thermometer api.

Thanks,
/Waldek


2013-02-20 07:57:25

by Johan Hedberg

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldek,

On Tue, Feb 12, 2013, Waldemar Rymarkiewicz wrote:
> Change-Id: I7165ecee0186dfdb51cd01838f28768d94ff12b9
> ---

Please provide a proper commit message.

> +Blood Pressure Manager hierarchy
> +================================
> +
> +Serviceorg.bluez
> +Interface org.bluez.BloodPressureManager1
> +Object path [variable prefix]/{hci0,hci1,...}
> +
> +Methods RegisterWatcher(object agent)
> +
> + Registers a watcher to monitor blood pressure
> + measurements. This agent will be notified about
> + final pressure measurements.
> +
> + Possible Errors: org.bluez.Error.InvalidArguments
> +
> + UnregisterWatcher(object agent)
> +
> + Unregisters a watcher.
> +
> + EnableIntermediateMeasurement(object agent)
> +
> + Enables intermediate measurement notifications
> + for this agent. Intermediate measurements will
> + be enabled only for devices which support it.
> +
> + Possible Errors: org.bluez.Error.InvalidArguments
> +
> + DisableIntermediateMeasurement(object agent)
> +
> + Disables intermediate measurement notifications
> + for this agent. It will disable notifications in
> + devices when the last agent removes the
> + watcher for intermediate measurements.
> +
> + Possible Errors: org.bluez.Error.InvalidArguments
> + org.bluez.Error.NotFound

I'm a bit confused, what's the difference between "watcher" and "agent"?
If they're the same thing just pick one term and stick to it.

> +Blood Pressure Profile hierarchy
> +================================
> +
> +Service org.bluez
> +Interface org.bluez.BloodPressure1
> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> +
> +Properties boolean Intermediate [readonly]
> +
> + True if the device supports intermediate
> + measurement notifications.

What's the use of this property (and the interface)? I don't see how it
relates to any other APIs in this specification.

Johan

2013-03-13 06:51:33

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi,

>>> +Interface org.bluez.BloodPressure1
>>> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>>> +
>>> +Methods RegisterWatcher(object watcher)
>>> +
>>> + Registers a watcher to monitor blood pressure
>>> + measurements. This watcher will be notified about
>>> + final pressure measurements.
>>> +
>>> + Possible Errors: org.bluez.Error.InvalidArguments
>>> +
>>> + UnregisterWatcher(object watcher)
>>> +
>>> + Unregisters a watcher.
>>> +
>>> + EnableIntermediateMeasurement(object watcher)
>>> +
>>> + Enables intermediate measurement notifications
>>> + for this watcher. Intermediate measurements will
>>> + be enabled only for devices which support it.
>>> +
>>> + Possible Errors: org.bluez.Error.InvalidArguments
>>> + org.bluez.Error.NotSupported
>>> +
>>> + DisableIntermediateMeasurement(object watcher)
>>> +
>>> + Disables intermediate measurement notifications
>>> + for this watcher. It will disable notifications in
>>> + devices when the last watcher is removed for
>>> + intermediate measurements.
>>> +
>>> + Possible Errors: org.bluez.Error.InvalidArguments
>>> + org.bluez.Error.NotFound
>>
>> why don't we make these two methods parts of a dict for RegisterWatcher. Enabling them manually seems rather pointless.
>
> It's been done intentionally to be able to return "not supported" error
> when intermediate measurement is not available.
>
> Having RegisterWatcher(object watcher, boolean intermediate) and
> additionally Intermediate property [read only] is an another option.
>
> In this case app will check intermediate property and set intermediate
> param to RegisterWatcher appropriately.
>

Any comments on this.

/Waldek

2013-03-06 08:51:54

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Marcel,

>> +Interface org.bluez.BloodPressure1
>> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>> +
>> +Methods RegisterWatcher(object watcher)
>> +
>> + Registers a watcher to monitor blood pressure
>> + measurements. This watcher will be notified about
>> + final pressure measurements.
>> +
>> + Possible Errors: org.bluez.Error.InvalidArguments
>> +
>> + UnregisterWatcher(object watcher)
>> +
>> + Unregisters a watcher.
>> +
>> + EnableIntermediateMeasurement(object watcher)
>> +
>> + Enables intermediate measurement notifications
>> + for this watcher. Intermediate measurements will
>> + be enabled only for devices which support it.
>> +
>> + Possible Errors: org.bluez.Error.InvalidArguments
>> + org.bluez.Error.NotSupported
>> +
>> + DisableIntermediateMeasurement(object watcher)
>> +
>> + Disables intermediate measurement notifications
>> + for this watcher. It will disable notifications in
>> + devices when the last watcher is removed for
>> + intermediate measurements.
>> +
>> + Possible Errors: org.bluez.Error.InvalidArguments
>> + org.bluez.Error.NotFound
>
> why don't we make these two methods parts of a dict for RegisterWatcher. Enabling them manually seems rather pointless.

It's been done intentionally to be able to return "not supported" error
when intermediate measurement is not available.

Having RegisterWatcher(object watcher, boolean intermediate) and
additionally Intermediate property [read only] is an another option.

In this case app will check intermediate property and set intermediate
param to RegisterWatcher appropriately.

/Waldek

2013-03-05 16:15:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldemar,

>>> +Blood Pressure Manager hierarchy
>>> +================================
>>> +
>>> +Serviceorg.bluez
>>> +Interface org.bluez.BloodPressureManager1
>>> +Object path [variable prefix]/{hci0,hci1,...}
>>> +
>>> +Methods RegisterWatcher(object agent)
>>
>> I do not really this manager business anymore. Splitting the interface over adapter and device object seems pointless. What I like to see is that we make one interface on the device object. We should have no proper lifetime rules for these.
>>
>> Basically a client using a specialised interface should have to deal with one entry point and not with multiple. Low Energy clients need to pick a device they want to operate with. So this should focus on device objects and nothing else.
>>
>
> Is this what you mean?
>
>
> diff --git a/doc/bloodpressure-api.txt b/doc/bloodpressure-api.txt
> new file mode 100644
> index 0000000..be18208
> --- /dev/null
> +++ b/doc/bloodpressure-api.txt
> @@ -0,0 +1,127 @@
> +Blood Pressure API description
> +******************************
> +
> +Blood Pressure Profile hierarchy
> +================================
> +
> +Service org.bluez
> +Interface org.bluez.BloodPressure1
> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> +
> +Methods RegisterWatcher(object watcher)
> +
> + Registers a watcher to monitor blood pressure
> + measurements. This watcher will be notified about
> + final pressure measurements.
> +
> + Possible Errors: org.bluez.Error.InvalidArguments
> +
> + UnregisterWatcher(object watcher)
> +
> + Unregisters a watcher.
> +
> + EnableIntermediateMeasurement(object watcher)
> +
> + Enables intermediate measurement notifications
> + for this watcher. Intermediate measurements will
> + be enabled only for devices which support it.
> +
> + Possible Errors: org.bluez.Error.InvalidArguments
> + org.bluez.Error.NotSupported
> +
> + DisableIntermediateMeasurement(object watcher)
> +
> + Disables intermediate measurement notifications
> + for this watcher. It will disable notifications in
> + devices when the last watcher is removed for
> + intermediate measurements.
> +
> + Possible Errors: org.bluez.Error.InvalidArguments
> + org.bluez.Error.NotFound

why don't we make these two methods parts of a dict for RegisterWatcher. Enabling them manually seems rather pointless.

Regards

Marcel


2013-03-05 11:39:17

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Marcel,

>> +Blood Pressure Manager hierarchy
>> +================================
>> +
>> +Serviceorg.bluez
>> +Interface org.bluez.BloodPressureManager1
>> +Object path [variable prefix]/{hci0,hci1,...}
>> +
>> +Methods RegisterWatcher(object agent)
>
> I do not really this manager business anymore. Splitting the interface over adapter and device object seems pointless. What I like to see is that we make one interface on the device object. We should have no proper lifetime rules for these.
>
> Basically a client using a specialised interface should have to deal with one entry point and not with multiple. Low Energy clients need to pick a device they want to operate with. So this should focus on device objects and nothing else.
>

Is this what you mean?


diff --git a/doc/bloodpressure-api.txt b/doc/bloodpressure-api.txt
new file mode 100644
index 0000000..be18208
--- /dev/null
+++ b/doc/bloodpressure-api.txt
@@ -0,0 +1,127 @@
+Blood Pressure API description
+******************************
+
+Blood Pressure Profile hierarchy
+================================
+
+Service org.bluez
+Interface org.bluez.BloodPressure1
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Methods RegisterWatcher(object watcher)
+
+ Registers a watcher to monitor blood pressure
+ measurements. This watcher will be notified about
+ final pressure measurements.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+
+ UnregisterWatcher(object watcher)
+
+ Unregisters a watcher.
+
+ EnableIntermediateMeasurement(object watcher)
+
+ Enables intermediate measurement notifications
+ for this watcher. Intermediate measurements will
+ be enabled only for devices which support it.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.NotSupported
+
+ DisableIntermediateMeasurement(object watcher)
+
+ Disables intermediate measurement notifications
+ for this watcher. It will disable notifications in
+ devices when the last watcher is removed for
+ intermediate measurements.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.NotFound
+
+
+Blood Pressure Watcher hierarchy
+================================
+
+Service unique name
+Interface org.bluez.BloodPressureWatcher1
+Object path freely definable
+
+Methods void MeasurementReceived(object device, dict measurement)
+
+ This callback is called whenever a blood pressure
+ measurement is received from the blood pressure device.
+
+ Measurement:
+
+ string Measurement:
+
+ Posible values: "final" or
+ "intermediate"
+
+ uint16 CurrentCuffPressure:
+
+ Current cuff pressure value is only
+ present in intermediate measurement.
+
+ uint16 Systolic:
+
+ Systolic Blood Pressure value is only
+ present in final measurement.
+
+ uint16 Diastolic:
+
+ Diastolic Blood Pressure value is only
+ present in final final measurement.
+
+ uint16 Mean:
+
+ Mean Arterial Pressure value is only
+ present in final measurement.
+
+ string Unit:
+
+ The unit of blood pressure measurement.
+
+ Possible values: "mmHg" or "kPa".
+
+ uint64 Time (optional):
+
+ Time of measurement expresed in seconds
+ from epoch.
+
+ uint16 PulseRate (optional):
+
+ Pulse rate value.
+
+ uint8 UserID (optional):
+
+ User ID if the device supports
+ multiusers measurements.
+
+ boolean BodyMovement (optional)
+
+ True if body movement during
+ measurement has been detected.
+
+ boolean CuffTooLoose (optional)
+
+ True if too loose cuff during
+ measurement has been detected.
+
+ boolean IrregularPulse (optional)
+
+ True if irrgular pulse during
+ measurement has been detected.
+
+ uint8 PulseRateNotInRange (optional)
+
+ Possible values:
+
+ 0 - pulse rate exceeds upper limit
+ 1 - pulse rate is less then lower limit
+
+ boolean MeasurementPosition (optional)
+
+ True if improper measurement position
+ has been detected.

2013-03-02 03:09:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldemar,

> Change-Id: I7165ecee0186dfdb51cd01838f28768d94ff12b9
> ---
> doc/bloodpressure-api.txt | 138 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 138 insertions(+)
> create mode 100644 doc/bloodpressure-api.txt
>
> diff --git a/doc/bloodpressure-api.txt b/doc/bloodpressure-api.txt
> new file mode 100644
> index 0000000..2acc660
> --- /dev/null
> +++ b/doc/bloodpressure-api.txt
> @@ -0,0 +1,138 @@
> +Blood Pressure API description
> +******************************
> +
> +
> +Blood Pressure Manager hierarchy
> +================================
> +
> +Serviceorg.bluez
> +Interface org.bluez.BloodPressureManager1
> +Object path [variable prefix]/{hci0,hci1,...}
> +
> +Methods RegisterWatcher(object agent)

I do not really this manager business anymore. Splitting the interface over adapter and device object seems pointless. What I like to see is that we make one interface on the device object. We should have no proper lifetime rules for these.

Basically a client using a specialised interface should have to deal with one entry point and not with multiple. Low Energy clients need to pick a device they want to operate with. So this should focus on device objects and nothing else.

Regards

Marcel


2013-03-01 13:37:33

by Johan Hedberg

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldek,

On Fri, Mar 01, 2013, Rymarkiewicz Waldemar wrote:
> >>>>+Blood Pressure Profile hierarchy
> >>>>+================================
> >>>>+
> >>>>+Service org.bluez
> >>>>+Interface org.bluez.BloodPressure1
> >>>>+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> >>>>+
> >>>>+Properties boolean Intermediate [readonly]
> >>>>+
> >>>>+ True if the device supports intermediate
> >>>>+ measurement notifications.
> >>
> >Couldn't you just return a "NotSupported" error to
> >EnableIntermediateMeasurement?
>
>
> I've just confronted this with the code and this make no sens
> because the watcher passed to EnableIntermediateMeasurement is
> registered on the adapter not for a device. So, it's not possible to
> check if the device supports this feature.
>
> Thus, I will keep Intermediate property for that reason.

Now it makes even less sense. If the EnableIntermediateMeasurement is
per adapter then what use is it to know about whether individual devices
support it or not? The method call will always succeed and the UI will
anyway need to be able to receive intermediate measurements from
arbitrary devices if it has called this method.

So what I'm lacking is a proper explanation of how a UI is supposed to
use this API (in a way that's actually meaningful) as well as why there
is this adapter/device split to begin with (i.e. why the registration
isn't per device or even bluetoothd global).

Johan

2013-03-01 12:11:15

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Johan,

>>>> +Blood Pressure Profile hierarchy
>>>> +================================
>>>> +
>>>> +Service org.bluez
>>>> +Interface org.bluez.BloodPressure1
>>>> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>>>> +
>>>> +Properties boolean Intermediate [readonly]
>>>> +
>>>> + True if the device supports intermediate
>>>> + measurement notifications.
>>
> Couldn't you just return a "NotSupported" error to
> EnableIntermediateMeasurement?


I've just confronted this with the code and this make no sens because
the watcher passed to EnableIntermediateMeasurement is registered on the
adapter not for a device. So, it's not possible to check if the device
supports this feature.

Thus, I will keep Intermediate property for that reason.

/Waldek


2013-03-01 08:39:39

by Johan Hedberg

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldek,

On Fri, Mar 01, 2013, Rymarkiewicz Waldemar wrote:
> Proposed 'NotSupported" seems fine as well. I can go for it and
> remove BloodPressure1 interface totally.
>
> Any more comments before I will send v2 ?

Nope, nothing more from me at least, though that's not a guarantee that
there wont be more feedback after your v2 :)

Johan

2013-03-01 08:34:08

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Johan,

> You might want to fix those in a separate patch set then. Just because
> something is in the source tree doesn't mean that it's worth copying
> into new code/documentation.

Will do so.

> Couldn't you just return a "NotSupported" error to
> EnableIntermediateMeasurement?

Sure, it could. It's rather cosmetic change then big one.

>> Similarly as in thermometer api.
>
> I suspect there are several messed up LE APIs and implementations in the
> source tree, so I'd say it's highly advisable to not just blindly copy
> from the existing code but also use some common sense and be critical of
> what you see.

I wouldn't say this particular api is messed up. For me it looks sane.
It's rather the matter of approach than blind copy/paste.

If something existing make sense and is pretty rational then should be
used. You can always code one thing in many ways which differ only in
details.

Proposed 'NotSupported" seems fine as well. I can go for it and remove
BloodPressure1 interface totally.

Any more comments before I will send v2 ?

Thanks,
/Waldek






2013-05-16 12:22:28

by Waldemar Rymarkiewicz

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Johan,

> Are you still working on this? Seems this discussion was forgotten or
> just abandoned.

I've been focused on something else, but nice to see you are still
interested in.

> Apparently it was never a requirement to be able to enable/disable this
> an arbitrary amount of times during the watcher life time, and instead
> just leave it disabled or enabled until the watcher gets unregistered?
>
> Also, do you think there will be UIs that will refuse to work if a
> device does not support intermediate measurement or that any UI that is
> interested in intermediate measurement will forgive devices that do not
> support it and simply get on with working without it being enabled? If
> the latter ilds true then why not make an API which allows requesting
>
> "register watcher and enable intermediate measurement if it's
> supported". In such a case you wouldn't need a separate property and the
> registration would succeed even if intermediate measurement isn't
> supported. Thoughts?


Well, I believe any app will always be interested in getting intermediate
results if possible, but it would be nice to notify (property, signal) the
app if it's not possible.
Just to give a chance to handle user experience in nice way. It can take a
while between start of measurements and the final results event.

From the other hand, the app by default can assume there is no intermediate
results ( (however API will not require that if there is no property)) and
can handle this time properly. In case, when it starts receiving
intermediate results it will start display it immediately.

Thanks,
/Waldek

2013-05-16 12:17:25

by Waldemar Rymarkiewicz

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Johan,

On 8 May 2013 09:41, Johan Hedberg <[email protected]> wrote:

> Are you still working on this? Seems this discussion was forgotten or
> just abandoned.
>

I've been focused on something else, but nice to see you are still
interested in.


> Apparently it was never a requirement to be able to enable/disable this
> an arbitrary amount of times during the watcher life time, and instead
> just leave it disabled or enabled until the watcher gets unregistered?
>
> Also, do you think there will be UIs that will refuse to work if a
> device does not support intermediate measurement or that any UI that is
> interested in intermediate measurement will forgive devices that do not
> support it and simply get on with working without it being enabled? If
> the latter ilds true then why not make an API which allows requesting
> "register watcher and enable intermediate measurement if it's
> supported". In such a case you wouldn't need a separate property and the
> registration would succeed even if intermediate measurement isn't
> supported. Thoughts?
>

Well, I believe any app will always be interested in getting intermediate
results if possible, but it would be nice to notify (property, signal) the
app if it's not possible.
Just to give a chance to handle user experience in nice way. It can take a
while between start of measurements and the final results event.

>From the other hand, the app by default can assume there is no intermediate
results ( (however API will not require that if there is no property)) and
can handle this time properly. In case, when it starts receiving
intermediate results it will start display it immediately.

Thanks,
/Waldek

2013-05-08 07:41:38

by Johan Hedberg

[permalink] [raw]
Subject: Re: [RFC] bloodpressure: Add Blood Pressure API

Hi Waldek,

On Wed, Mar 13, 2013, Rymarkiewicz Waldemar wrote:
>>>>+Interface org.bluez.BloodPressure1
>>>>+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>>>>+
>>>>+Methods RegisterWatcher(object watcher)
>>>>+
>>>>+ Registers a watcher to monitor blood pressure
>>>>+ measurements. This watcher will be notified about
>>>>+ final pressure measurements.
>>>>+
>>>>+ Possible Errors: org.bluez.Error.InvalidArguments
>>>>+
>>>>+ UnregisterWatcher(object watcher)
>>>>+
>>>>+ Unregisters a watcher.
>>>>+
>>>>+ EnableIntermediateMeasurement(object watcher)
>>>>+
>>>>+ Enables intermediate measurement notifications
>>>>+ for this watcher. Intermediate measurements will
>>>>+ be enabled only for devices which support it.
>>>>+
>>>>+ Possible Errors: org.bluez.Error.InvalidArguments
>>>>+ org.bluez.Error.NotSupported
>>>>+
>>>>+ DisableIntermediateMeasurement(object watcher)
>>>>+
>>>>+ Disables intermediate measurement notifications
>>>>+ for this watcher. It will disable notifications in
>>>>+ devices when the last watcher is removed for
>>>>+ intermediate measurements.
>>>>+
>>>>+ Possible Errors: org.bluez.Error.InvalidArguments
>>>>+ org.bluez.Error.NotFound
>>>
>>> why don't we make these two methods parts of a dict for
>>> RegisterWatcher. Enabling them manually seems rather pointless.
>>
>> It's been done intentionally to be able to return "not supported" error
>> when intermediate measurement is not available.
>>
>> Having RegisterWatcher(object watcher, boolean intermediate) and
>> additionally Intermediate property [read only] is an another option.
>>
>> In this case app will check intermediate property and set intermediate
>> param to RegisterWatcher appropriately.
>
> Any comments on this.

Are you still working on this? Seems this discussion was forgotten or
just abandoned.

Having the intermediate measurement enabling in the registration instead
of a separate method and you being fine with it (assuming that an
Intermediate property is added) raises some questions.

Apparently it was never a requirement to be able to enable/disable this
an arbitrary amount of times during the watcher life time, and instead
just leave it disabled or enabled until the watcher gets unregistered?

Also, do you think there will be UIs that will refuse to work if a
device does not support intermediate measurement or that any UI that is
interested in intermediate measurement will forgive devices that do not
support it and simply get on with working without it being enabled? If
the latter is true then why not make an API which allows requesting
"register watcher and enable intermediate measurement if it's
supported". In such a case you wouldn't need a separate property and the
registration would succeed even if intermediate measurement isn't
supported. Thoughts?

Johan