Opened 8 years ago

Closed 6 years ago

#1303 closed defect (fixed)

olpc-hardware-manager in C

Reported by: jochang Owned by: cjb
Priority: normal Milestone: Opportunity
Component: distro Version:
Keywords: Cc: hugsient@…, reynaldo@…
Blocked By: Blocking:
Deployments affected: Action Needed: never set
Verified: no

Description

Attached is a rewrite of olpc-hardware-manager in C (It was a 130 lines Python script). This saves about 1.8M private dirty memory according to /proc/.../smaps. I tested it on my B2 and it works fine. I hope this makes sense as this is my
first time using dbus.

Attachments (5)

hardwaremanager.c (5.8 KB) - added by jochang 8 years ago.
Makefile (186 bytes) - added by jochang 8 years ago.
hardwaremanager.2.c (7.4 KB) - added by jochang 8 years ago.
add introspection support.
hardwaremanager.2-nokbdlight.c (6.2 KB) - added by reynaldo 7 years ago.
Self explanatory, gets rid of keyboard LEDs related code, haven't tested but guess it works -- what else needs to be done here?
hardwaremanager.2.c-getsridofkbdLEDS.diff (2.1 KB) - added by reynaldo 7 years ago.
hardwaremanager.2-nokbdlight.c as a patch against hardwaremanager.2.c. Just in case you need to review the changes quickly

Download all attachments as: .zip

Change History (18)

Changed 8 years ago by jochang

Changed 8 years ago by jochang

comment:1 Changed 8 years ago by marco

  • Cc dcbw added

Dan, can you have a look to this please? /me never used dbus in C

comment:2 Changed 8 years ago by jg

  • Milestone changed from Untriaged to BTest-3

comment:3 Changed 8 years ago by dcbw

Nice! Looks pretty good. Two comments though:

1) The timeout should be shorter for dbus_connection_read_write_dispatch(); say, 5 seconds perhaps. If it's -1, the program will block in the select/poll for quite a long time and won't exit when sent a SIGTERM for that amount of time. You can see this with dbus-monitor. Ideally when sent a SIGTERM (on system shutdown, for instance) it should shut down pretty quickly.

2) We need introspection support too; this means slightly more complicated code, since you have to deal with the "Introspect" method call. But since the service isn't supposed to be that complex, you can definitely hardcode the Introspection XML, read it in at the start of the program, and just dump it back out when you get a request for Introspect.

Thanks again; if you've got a little time, could you take a pass at doing these two things as well? If you need any help or pointers, let us know.

Changed 8 years ago by jochang

add introspection support.

comment:4 Changed 8 years ago by jochang

Thanks for the review!

1) I think the default handler of SIGTERM should terminate the process immediately (just tested).

2) Introspection is added in the new attachment. I hardcoded the XML in the C file because it is small and won't change unless the C code changes also.

comment:5 Changed 8 years ago by jg

  • Cc dcbw removed
  • Owner changed from blizzard to dcbw
  • Verified unset

Dan, is this ready to merge? If so, assign to J5.

If not, what remains?

comment:6 Changed 8 years ago by kimquirk

  • Milestone changed from BTest-4 to Trial-2

Moving to next software release, Trial-2.

comment:7 Changed 7 years ago by jg

  • Cc hugsient@… added
  • Milestone changed from Trial-2 to Trial-3
  • Owner changed from dcbw to cjb

What's the state of this vis-a-vis OHM?

comment:8 Changed 7 years ago by marco

These are the keys which we are setting with hardware-manager atm.

_KEYBOARD_BRIGHTNESS_KEY = '/sys/class/leds/olpc:keyboard/brightness'

Is this in the latest hardware at all?

_DISPLAY_BRIGHTNESS_KEY = '/sys/class/backlight/dcon-bl/brightness'

Can we use ohm for this?

_DISPLAY_MODE_KEY = '/sys/devices/platform/dcon/output'

Black white/color, could this be added to ohm?

_DCON_SOURCE_KEY = '/sys/devices/platform/dcon/source'
_DCON_FREEZE_KEY = '/sys/devices/platform/dcon/freeze'

Not sure about these.

_POWER_STATE_KEY = '/sys/power/state'

Is this handled by ohm already?

comment:9 Changed 7 years ago by warp

We can not at this time handle keys that come over the keyboard channel via anything that doesn't work through X.

This means things like the brightness keys are going to have to stick with suger a bit longer.

This is a known limitation of the current grabbing method, and work is in progress to replace it with something better, but we'll see what happens.

(If I'm misunderstanding where the key grabbing for this would be happening, my apologies.)

comment:10 Changed 7 years ago by marco

I was thinking more about having sugar call some dbus method in ohm to change brightness (similarly to what we do right now with olpc-hardware-manager). Though, if the long term plan is to handle this outside sugar maybe it's just worth waiting that happens.

comment:11 Changed 7 years ago by walter

  • Milestone changed from Untriaged to Opportunity (please help!)

Changed 7 years ago by reynaldo

Self explanatory, gets rid of keyboard LEDs related code, haven't tested but guess it works -- what else needs to be done here?

Changed 7 years ago by reynaldo

hardwaremanager.2-nokbdlight.c as a patch against hardwaremanager.2.c. Just in case you need to review the changes quickly

comment:12 Changed 7 years ago by reynaldo

  • Cc reynaldo@… added

comment:13 Changed 6 years ago by cjb

  • Action Needed set to never set
  • Resolution set to fixed
  • Status changed from new to closed

Replaced by OHM, closing.

Note: See TracTickets for help on using tickets.