Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12529 - Lamp plugins PLA integration

Attached to Project: Rockbox
Opened by Jean-Louis Biasini (JeanLouisBiasini) - Tuesday, 10 January 2012, 15:29 GMT+2
Last edited by Thomas Martitz (kugel.) - Wednesday, 08 February 2012, 18:09 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.10
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

this is a try for a PLA integration of lamp.
The purpose is to simplify manual and code keymaps

note:
- ONDAVX747(poorly) and MROBE500 added to PLA
- IAUDIO67 PLA keymaps corrected (2 command were mapped to the same keys)
- exit lamp mapped to PLA_EXIT and PLA_CANCEL
   lamp-PLA-integration-v1.diff (9.6 KiB) 
 apps/plugins/lib/pluginlib_actions.c |   13 +-
 apps/plugins/lamp.c                  |  161 ++++++++---------------------------
 2 files changed, 46 insertions(+), 128 deletions(-)

This task depends upon

Closed by  Thomas Martitz (kugel.)
Wednesday, 08 February 2012, 18:09 GMT+2
Reason for closing:  Out of Date
Additional comments about closing:  http://gerrit.rockbox.org/r/#change,86
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Tuesday, 10 January 2012, 15:48 GMT+2
gevaerts pointed out that mr500 get handled through touchscreen interface
   lamp-PLA-integration-v2.diff (9.1 KiB) 
 apps/plugins/lib/pluginlib_actions.c |    9 +
 apps/plugins/lamp.c                  |  161 ++++++++---------------------------
 2 files changed, 43 insertions(+), 127 deletions(-)

Comment by Thomas Martitz (kugel.) - Wednesday, 11 January 2012, 08:48 GMT+2
Your changes to pluginlib_actions.c are not clear to me. Do you own the targets so you can tell it's an improvement? Changes to PLA should probably be a separate task.

The changes to lamp look fine at first sight.
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Wednesday, 11 January 2012, 10:14 GMT+2
- ondavx747 was not impleted in PLA. Unless it's a touchscreen device, (in which case I have to revert this part as I did for the mr500) it definitivly do no harm to have it in PLA.
- iaudio67 is a dead port, but the same key was mapped both to down and select keys. this cannot works. The only remaiming key not mapped is the one I linked as a remplacement
Comment by Thomas Martitz (kugel.) - Wednesday, 11 January 2012, 10:21 GMT+2
ondavx747 is a touchscreen device. You can see such things by looking at firmware/export/config/<target>.h btw
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Thursday, 12 January 2012, 19:06 GMT+2
ok corrected and I removed cowon2 also as it is also touchscreen
   lamp-PLA-integration-v3.diff (8.4 KiB) 
 apps/plugins/lib/pluginlib_actions.c |    6 -
 apps/plugins/lamp.c                  |  158 +++++++----------------------------
 2 files changed, 35 insertions(+), 129 deletions(-)

Comment by amaury pouly (pamaury) - Sunday, 15 January 2012, 01:41 GMT+2
Is this ready to commit ? I know nothing about plugin actions so I would prefer that someone review it before.
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Sunday, 15 January 2012, 14:19 GMT+2
I guess this is ready but it would be nice to have kugel's opinion on this
Comment by Thomas Martitz (kugel.) - Sunday, 15 January 2012, 14:23 GMT+2
It *looks* fine to me, but I haven't tested.

Does it compile for HAVE_SCROLLWHEEL? LAMP_(UP|DOWN)_REPEAT doesn't seem defined for those.
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Sunday, 15 January 2012, 14:54 GMT+2
No I just tried!
Ok now this compile
   lamp-PLA-integration-v4.diff (8.5 KiB) 
 apps/plugins/lib/pluginlib_actions.c |    6 -
 apps/plugins/lamp.c                  |  160 +++++++----------------------------
 2 files changed, 37 insertions(+), 129 deletions(-)

Comment by Thomas Martitz (kugel.) - Wednesday, 25 January 2012, 22:27 GMT+2
The block is not clear to me:


#ifdef HAVE_TOUCHSCREEN
# ifndef LAMP_LEFT
# define LAMP_LEFT PLA_LEFT
# endif
# ifndef LAMP_RIGHT
# define LAMP_RIGHT PLA_RIGHT
# endif
# ifndef LAMP_UP
# define LAMP_UP PLA_UP
# endif
# ifndef LAMP_DOWN
# define LAMP_DOWN PLA_DOWN
# endif
#endif

What's the purpose. Isn't touchscreen already covered by the above? (touchscreen assigns grid mode buttons for PLA so all buttons are already defined).

Otherwise it looks good. However I would prefer if you'd also upload a patch for the manual.
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Saturday, 28 January 2012, 17:07 GMT+2
This block was also in the original part so I just translated it into PLA. I have to check if I can take it out. Ok I will add the manual patch also
Comment by Thomas Martitz (kugel.) - Saturday, 28 January 2012, 18:20 GMT+2
I see. I can be removed with the transition to PLA.
Comment by Jean-Louis Biasini (JeanLouisBiasini) - Monday, 06 February 2012, 15:09 GMT+2
Touchscreen block has been taken away: (compile and works on robe500 simulatorui)
http://gerrit.rockbox.org/r/#change,86

Manual update's patch:
http://gerrit.rockbox.org/r/#change,87

Loading...