[PATCH] add support for ACPI IBM hotkey events

David Zeuthen david at fubar.dk
Fri Mar 30 12:14:52 PDT 2007


On Thu, 2007-03-29 at 22:34 +0200, Danny Kukawka wrote:
> > Yes please. I listed a list of things I'd like to see in my other
> mail.
> >
> >  http://lists.freedesktop.org/archives/hal/2007-March/007778.html
> 
> I hope the attached version now contains everything. I would rename 
> 10-toshiba-buttons.fdi to 10-laptop-buttons.fdi to have not several files with 
> only some rules and both entries do at least the same.

Uhm, please don't commit non-trivial code before it's ACK'ed (and I
realize I should have responded sooner; sorry about that.)

> +AC_SUBST(ACPI_IBM_EVENTS)

Probably better to call it BUILD_ACPI_IBM

> +        catch IBM acpi events:      ${acpi_ibm} (Linux only)

Needs to start with capital letter. Also, it's bad language; should
probably be "Forward IBM acpi events". Ditto where AC_ARG_ENABLED is
used. Same for the Toshiba patch. Also ACPI should be in capital
letters.

> +                       case 4100: /* sleep button */
> +                               button = "sleep";
> +                               devices =
> libhal_manager_find_device_string_match (ctx, "button.type",
> +
> "sleep", &num_devices,
> +
> &error);
> +                               if (devices != NULL && num_devices >
> 0) {
> +                                       snprintf (udi, sizeof (udi),
> devices[0]);
> +                                       libhal_free_string_array
> (devices);
> +                               }
> +                               if (dbus_error_is_set (&error)) 
> +                                       dbus_error_free (&error);

I said in the link above that this needs to go.

Also, IBM and Toshiba hotkey handling cannot share the same fdi file
because they may not be enabled at the same time. Also, you need to only
install the fdi file when we build with this option enabled.

Finally, configure.in should print warnings like this

NOTE: Future Linux kernels may provide IBM ACPI events via a kernel.
      input device resulting in HAL emitting twice as many events as it
      should. Make sure to rebuild HAL without --enable-acpi-ibm if
      you use such a kernel.

NOTE: Future Linux kernels may provide Toshiba ACPI events via a kernel.
      input device resulting in HAL emitting twice as many events as it
      should. Make sure to rebuild HAL without --enable-acpi-toshiba if
      you use such a kernel.

I've fixed all this with this commit:

http://gitweb.freedesktop.org/?p=hal.git;a=commitdiff;h=992dbc1929d036195d144e8051fddfd1e5c71779

      David




More information about the hal mailing list