Re: [PATCH v4 01/10] Battery: Add Battery Service Gatt Client

Hi Chen,

On Thu, Aug 16, 2012, Chen Ganir wrote:

quoted

I don't think it's ok to pollute the Device interface or src/device.c
with profile-specific details. That should happen in profile-specific
plugins and interfaces. Since we're switching over to object manager
maybe an interface/property like this isn't needed at all? (even if it
would be needed the type should be array{object} and not array{string}

You are correct, the array should be object instead of string.
However, i fail to understand why the object manager prevents this
from getting accepted - do you have a time estimation when the
object manager should be active or available ?

By the time of our next release (as per the content under the BlueZ 5
section in our TODO file).

I prefer this way of putting the batteries below the device, since it
is obvious which battery belongs to each device.

I fully agree with this. It's not what I had an issue with. However, if
we really want to make this clear you could additionally have a "Device"
property for each battery object pointing back to the parent object.

The other option i can think of is to have another interface
registered on the device object path:

Service		org.bluez
Interface	org.bluez.Batteries
Object path	[variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX

Methods	dict GetProperties()

		Returns all properties for the interface. See the
		Properties section for the available properties.

Signals		ProperyChanged(string name, variant value)

		This signal indicates a changed value of the given
		property.

Properties	array{object} Batteries [readonly]

		List of device battery object paths that represents the available
batteries on the remote devices.


Service		org.bluez
Interface	org.bluez.Battery
Object path	[variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX/BATT-NN-DDDD


Methods	dict GetProperties()

		Returns all properties for the interface. See the
		Properties section for the available properties.

Signals		PropertyChanged(string name, variant value)

		This signal indicates a changed value of the given
		property.

Properties	byte Level [readonly]

			Battery level (0-100).

Any other suggestion ?

That would work, although both of these interfaces are rather
light-weight in that they only contain a single property which begs the
question whether this is a bit over-kill.

Another option is to do away with the per-battery object somehow and
expose all batteries through a single interface. I'm not so sure that's
a good idea though since it could prevent future extensibility of the
API and doesn't reuse the common tools we have for object-like
abstractions (which a battery certainly is).

Johan