[Sugar-devel] [PATCH] Touchpad icon: update for upstream kernel API

[Daniel Drake]

On Mon, Nov 28, 2011 at 7:51 AM, Sascha Silbe <silbe at activitycentral.com> wrote:
>> Port the touchpad icon to this new API.
>
> You're doing more than this. The additional changes are fine, but should
> be mentioned.

I've checked again and I don't see any changes in the patch other than
porting the touchpad icon to the new kernel interface.

>> Compatibility with the old non-upstream API has been dropped; I don't
>> anticipate users running this new sugar with an old kernel.
>
> FWIW, I'm still running a 2.6.35 kernel on my XO-1 (but Sugar from
> sugar-jhbuild, i.e. mainline/master). Not sure if anyone who cares about
> this Frame device icon does, though.

For simplicity I think we should go ahead and merge this despite
breaking uncommon use cases.

> [extensions/deviceicon/touchpad.py]
>> @@ -31,19 +31,15 @@ from sugar.graphics import style
>>
>>  from jarabe.frame.frameinvoker import FrameWidgetInvoker
>>
>> -TOUCHPAD_MODE_CAPACITIVE = 'capacitive'
>> -TOUCHPAD_MODE_RESISTIVE = 'resistive'
>> -TOUCHPAD_MODES = [TOUCHPAD_MODE_CAPACITIVE, TOUCHPAD_MODE_RESISTIVE]
>> -STATUS_TEXT = {
>> -    TOUCHPAD_MODE_CAPACITIVE: _('finger'),
>> -    TOUCHPAD_MODE_RESISTIVE: _('stylus'),
>> -}
>> -STATUS_ICON = {
>> -    TOUCHPAD_MODE_CAPACITIVE: 'touchpad-' + TOUCHPAD_MODE_CAPACITIVE,
>> -    TOUCHPAD_MODE_RESISTIVE: 'touchpad-' + TOUCHPAD_MODE_RESISTIVE,
>> -}
>
>> +TOUCHPAD_MODE_MOUSE = 'mouse'
>> +TOUCHPAD_MODE_PENTABLET = 'pentablet'
>
> Do we need these as individual constants? I don't see them used anywhere
> except when defining TOUCHPAD_MODES and since we seem to change the name
> of the constants any time the content changes, I see no reason to use
> them at all. Especially since we don't do the same for STATUS_TEXT and
> STATUS_ICON:

The constants are used elsewhere - take another look. It also follows
the earlier design which had e.g. TOUCHPAD_MODE_CAPACITIVE. Does that
change your opinion?

> Typo: s/indice/index/
> I find "if a not in b" easier to read than "if not a in b"; YMMV.

If  these are the only real issues that you have identified then I
would be happy to fix them up at commit time.

Daniel