[Sugar-devel] [PATCH sugar v2] battery frame device: replace HAL with UPower
Simon Schampijer
simon at schampijer.de
Fri Feb 18 12:32:51 EST 2011
Hi Sascha,
thanks for that patch.
On 01/21/2011 10:35 AM, Sascha Silbe wrote:
> HAL is deprecated, UPower is the designated replacement w.r.t. power
> supplies. As a bonus the time displayed is now correct (calculated at
> run-time by UPower, self-adjusting to changes in power consumption).
Ok, I was first put off by the new behavior but I guess it is a good
thing in the end. We should maybe note it in the release notes as a Feature.
> Tested on XO-1.5 running Debian Squeeze with stock UPower (0.9.5-5) and a
> 2.6.35 OLPC kernel.
I have tested the patch on the latest 11.2.0 build as working fine.
The overall patch looks great - a few smaller comments inline.
> Signed-off-by: Sascha Silbe<sascha-pgp at silbe.org>
> ---
> v1->v2: Rebased on mainline/master
>
> extensions/deviceicon/battery.py | 194 +++++++++++++++++++-------------------
> 1 files changed, 98 insertions(+), 96 deletions(-)
>
> diff --git a/extensions/deviceicon/battery.py b/extensions/deviceicon/battery.py
> index 4028878..0d7381e 100644
> --- a/extensions/deviceicon/battery.py
> +++ b/extensions/deviceicon/battery.py
> @@ -16,8 +16,9 @@
>
> import logging
> from gettext import gettext as _
> -import gconf
> +import sys
>
> +import gconf
> import gobject
> import gtk
> import dbus
> @@ -38,17 +39,24 @@ _STATUS_DISCHARGING = 1
> _STATUS_FULLY_CHARGED = 2
> _STATUS_NOT_PRESENT = 3
>
> -_LEVEL_PROP = 'battery.charge_level.percentage'
> -_CHARGING_PROP = 'battery.rechargeable.is_charging'
> -_DISCHARGING_PROP = 'battery.rechargeable.is_discharging'
> -_PRESENT_PROP = 'battery.present'
> +_UP_TYPE_BATTERY = 2
> +
> +_UP_STATE_UNKNOWN = 0
> +_UP_STATE_CHARGING = 1
> +_UP_STATE_DISCHARGING = 2
> +_UP_STATE_EMPTY = 3
> +_UP_STATE_FULL = 4
> +_UP_STATE_CHARGE_PENDING = 5
> +_UP_STATE_DISCHARGE_PENDING = 6
> +
> +_WARN_MIN_PERCENTAGE = 15
>
>
> class DeviceView(TrayIcon):
>
> FRAME_POSITION_RELATIVE = 102
>
> - def __init__(self, udi):
> + def __init__(self, battery):
> client = gconf.client_get_default()
> self._color = XoColor(client.get_string('/desktop/sugar/user/color'))
>
> @@ -56,17 +64,10 @@ class DeviceView(TrayIcon):
>
> self.set_palette_invoker(FrameWidgetInvoker(self))
>
> - self._model = DeviceModel(udi)
> + self._model = DeviceModel(battery)
> self.palette = BatteryPalette(_('My Battery'))
> self.palette.set_group_id('frame')
> -
> - self._model.connect('notify::level',
> - self._battery_status_changed_cb)
> - self._model.connect('notify::charging',
> - self._battery_status_changed_cb)
> - self._model.connect('notify::discharging',
> - self._battery_status_changed_cb)
> - self._model.connect('notify::present',
> + self._model.connect('updated',
> self._battery_status_changed_cb)
If we touch the code already, maybe make it private, two '__'.
> self._update_info()
>
> @@ -88,7 +89,7 @@ class DeviceView(TrayIcon):
> style.COLOR_WHITE.get_svg()))
> elif self._model.props.discharging:
> status = _STATUS_DISCHARGING
> - if current_level<= 15:
> + if current_level<= _WARN_MIN_PERCENTAGE:
> badge_name = 'emblem-warning'
> else:
> status = _STATUS_FULLY_CHARGED
> @@ -98,10 +99,10 @@ class DeviceView(TrayIcon):
> self.icon.props.xo_color = xo_color
> self.icon.props.badge_name = badge_name
>
> - self.palette.set_level(current_level)
> - self.palette.set_status(status)
> + self.palette.set_info(current_level, self._model.props.time_remaining,
> + status)
>
> - def _battery_status_changed_cb(self, pspec, param):
> + def _battery_status_changed_cb(self, *args):
> self._update_info()
>
>
> @@ -109,8 +110,9 @@ class BatteryPalette(Palette):
>
> def __init__(self, primary_text):
> Palette.__init__(self, primary_text)
> -
> self._level = 0
> + self._time = 0
> + self._status = _STATUS_NOT_PRESENT
> self._progress_bar = gtk.ProgressBar()
> self._progress_bar.set_size_request(
> style.zoom(style.GRID_CELL_SIZE * 4), -1)
> @@ -126,29 +128,29 @@ class BatteryPalette(Palette):
> self._progress_widget = vbox
> self.set_content(self._progress_widget)
>
> - def set_level(self, percent):
> + def set_info(self, percent, seconds, status):
> self._level = percent
> - fraction = percent / 100.0
> - self._progress_bar.set_fraction(fraction)
> + self._time = seconds
> + self._status = status
> + self._progress_bar.set_fraction(percent / 100.0)
> + self._update_secondary()
>
> - def set_status(self, status):
> - current_level = self._level
> + def _update_secondary(self):
> secondary_text = ''
> - status_text = '%s%%' % current_level
> + status_text = '%s%%' % self._level
>
> progress_widget = self._progress_widget
> - if status == _STATUS_NOT_PRESENT:
> + if self._status == _STATUS_NOT_PRESENT:
> secondary_text = _('Removed')
> progress_widget = None
> - elif status == _STATUS_CHARGING:
> + elif self._status == _STATUS_CHARGING:
> secondary_text = _('Charging')
> - elif status == _STATUS_DISCHARGING:
> - if current_level<= 15:
> + elif self._status == _STATUS_DISCHARGING:
> + if self._level<= _WARN_MIN_PERCENTAGE:
> secondary_text = _('Very little power remaining')
> else:
> - #TODO: make this less of an wild/educated guess
> - minutes_remaining = int(current_level / 0.59)
> - remaining_hourpart = minutes_remaining / 60
> + minutes_remaining = self._time // 60
> + remaining_hourpart = minutes_remaining // 60
> remaining_minpart = minutes_remaining % 60
> secondary_text = _('%(hour)d:%(min).2d remaining') % \
> {'hour': remaining_hourpart, 'min': remaining_minpart}
> @@ -163,91 +165,91 @@ class BatteryPalette(Palette):
> class DeviceModel(gobject.GObject):
> __gproperties__ = {
> 'level': (int, None, None, 0, 100, 0, gobject.PARAM_READABLE),
> + 'time-remaining': (int, None, None, 0, sys.maxint, 0,
> + gobject.PARAM_READABLE), # unit: seconds
> 'charging': (bool, None, None, False, gobject.PARAM_READABLE),
> 'discharging': (bool, None, None, False, gobject.PARAM_READABLE),
> 'present': (bool, None, None, False, gobject.PARAM_READABLE),
> }
>
> - def __init__(self, udi):
> - gobject.GObject.__init__(self)
> -
> - bus = dbus.Bus(dbus.Bus.TYPE_SYSTEM)
> - proxy = bus.get_object('org.freedesktop.Hal', udi,
> - follow_name_owner_changes=True)
> - self._battery = dbus.Interface(proxy, 'org.freedesktop.Hal.Device')
> - bus.add_signal_receiver(self._battery_changed,
> - 'PropertyModified',
> - 'org.freedesktop.Hal.Device',
> - 'org.freedesktop.Hal',
> - udi)
> -
> - self._level = self._get_level()
> - self._charging = self._get_charging()
> - self._discharging = self._get_discharging()
> - self._present = self._get_present()
> -
> - def _get_level(self):
> - try:
> - return self._battery.GetProperty(_LEVEL_PROP)
> - except dbus.DBusException:
> - logging.error('Cannot access %s', _LEVEL_PROP)
> - return 0
> -
> - def _get_charging(self):
> - try:
> - return self._battery.GetProperty(_CHARGING_PROP)
> - except dbus.DBusException:
> - logging.error('Cannot access %s', _CHARGING_PROP)
> - return False
> + __gsignals__ = {
> + 'updated': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, ([])),
> + }
>
> - def _get_discharging(self):
> + def __init__(self, battery):
> + gobject.GObject.__init__(self)
> + self._battery = battery
> + self._battery_props_iface = dbus.Interface(self._battery,
> + dbus.PROPERTIES_IFACE)
> + self._battery.connect_to_signal('Changed', self._battery_changed,
> + dbus_interface='org.freedesktop.UPower.Device')
For the callback name I would use self.__battery_changed_cb
> + self._fetch()
> + def _fetch(self):
Why not make it _get_battery_properties ?
> + """Get current values from UPower."""
> + # pylint: disable-msg=W0201
> try:
> - return self._battery.GetProperty(_DISCHARGING_PROP)
> + dbus_props = self._battery_props_iface.GetAll(
> + 'org.freedesktop.UPower.Device')
> except dbus.DBusException:
> - logging.error('Cannot access %s', _DISCHARGING_PROP)
> - return False
> + logging.error('Cannot access battery properties')
> + dbus_props = {}
>
> - def _get_present(self):
> - try:
> - return self._battery.GetProperty(_PRESENT_PROP)
> - except dbus.DBusException:
> - logging.error('Cannot access %s', _PRESENT_PROP)
> - return False
> + self._level = dbus_props.get('Percentage', 0)
> + self._state = dbus_props.get('State', _UP_STATE_UNKNOWN)
> + self._present = dbus_props.get('IsPresent', False)
> + self._time_to_empty = dbus_props.get('TimeToEmpty', 0)
> + self._time_to_full = dbus_props.get('TimeToFull', 0)
>
> def do_get_property(self, pspec):
> + """Return current value of given GObject property."""
> if pspec.name == 'level':
> return self._level
> if pspec.name == 'charging':
> - return self._charging
> + return self._state == _UP_STATE_CHARGING
> if pspec.name == 'discharging':
> - return self._discharging
> + return self._state == _UP_STATE_DISCHARGING
> if pspec.name == 'present':
> return self._present
> + if pspec.name == 'time-remaining':
> + if self._state == _UP_STATE_CHARGING:
> + return self._time_to_full
> + if self._state == _UP_STATE_DISCHARGING:
> + return self._time_to_empty
> + return 0
>
> def get_type(self):
> return 'battery'
>
> - def _battery_changed(self, num_changes, changes_list):
> - for change in changes_list:
> - if change[0] == _LEVEL_PROP:
> - self._level = self._get_level()
> - self.notify('level')
> - elif change[0] == _CHARGING_PROP:
> - self._charging = self._get_charging()
> - self.notify('charging')
> - elif change[0] == _DISCHARGING_PROP:
> - self._discharging = self._get_discharging()
> - self.notify('discharging')
> - elif change[0] == _PRESENT_PROP:
> - self._present = self._get_present()
> - self.notify('present')
> + def _battery_changed(self, *args):
> + old_level = self._level
> + old_state = self._state
> + old_present = self._present
> + old_time = self.props.time_remaining
> + self._fetch()
> + if self._level != old_level:
> + self.notify('level')
> + if self._state != old_state:
> + self.notify('charging')
> + self.notify('discharging')
> + if self._present != old_present:
> + self.notify('present')
> + if self.props.time_remaining != old_time:
> + self.notify('time-remaining')
> +
> + self.emit('updated')
>
>
> def setup(tray):
> bus = dbus.Bus(dbus.Bus.TYPE_SYSTEM)
> - proxy = bus.get_object('org.freedesktop.Hal',
> - '/org/freedesktop/Hal/Manager')
> - hal_manager = dbus.Interface(proxy, 'org.freedesktop.Hal.Manager')
> -
> - for udi in hal_manager.FindDeviceByCapability('battery'):
> - tray.add_device(DeviceView(udi))
> + up_proxy = bus.get_object('org.freedesktop.UPower',
> + '/org/freedesktop/UPower')
> + upower = dbus.Interface(up_proxy, 'org.freedesktop.UPower')
> +
> + for battery_path in upower.EnumerateDevices():
I would not call it battery_path here as you are enumerating devices,
hence device_path (of course you need to follow this as well in the code
below).
> + battery = bus.get_object('org.freedesktop.UPower', battery_path)
> + batt_prop_iface = dbus.Interface(battery, dbus.PROPERTIES_IFACE)
> + device_type = batt_prop_iface.Get('org.freedesktop.UPower.Device',
> + 'Type')
> + if device_type == _UP_TYPE_BATTERY:
> + tray.add_device(DeviceView(battery))
Regards,
Simon
More information about the Sugar-devel
mailing list