Rockbox

Wouldn't SELECT and CANCEL make more sense than START and QUIT, and why shouldn't every button have a repeat?

Why should it make more sense? It totally depends on the plugin what the key actually does. And most of the existing PLA plugins use it for starting or exiting the "content".

Quit maps to the same button as Power off for many devices, so I rather not have repeat for that one. It will just conflict, so I rather not encourage repeat on that one.
But that's also a problem I don't address and which exists already. I actually added repeat to most others.

It makes more sense because "Start" and "Quit" are specific while "select" and "cancel" are general. We refer to a button in almost every manual as "select" and it will almost always be the "select" action. The button doesn't *start* things very often. In fact, in most plugins it's simply used to select menu entries.

As for repeating on the "Quit//Cancel" repeating, we allow (and commonly use) it in the core, so it's rather unreasonable to restrict it in plugins.

As well, unless the button is *only* going to be used to quit plugins, "cancel" is a better term for it, because you can cancel out of the plugin, or you can cancel selection. They're both just more general terms than "Start" and "Quit"

Well, OK for cancel/select, I don't care much.

However
"As for repeating on the "Quit//Cancel" repeating, we allow (and commonly use) it in the core, so it's rather unreasonable to restrict it in plugins." is not true for most targets.

Cancel in the core is almost never mapped to power off (instead, it's commonly mapped to BUTTON_LEFT), while this PLA_QUIT is mapped to the power off button for most targets (because BUTTON_LEFT is already in use for PLA_LEFT). So, in the core there's no problem, but for PLA there is.

And once more, this isn't an issue my patch covers at all.

Yes, but we also allow repeat on the power off button, so focusing on the cancel button doesn't change that.

That's only allowed for targets which are already short on buttons, which are not that much (ondio and ipods, any more?). And I don't want to encourage this.
The buttons that are offered are sufficient so far.

Alright - how many targets have a hardware power off on a button that *has* to be used for "Cancel" because there aren't enough other buttons, exactly, and the hardware power off is in a short enough time (say, less than 5-7 seconds) to be dangerous?

This is the _only_ case where repeating cancel is bad - when cancel needs to be assigned the same button as a hardware power off.

Even on the e200 the hardware power off is quite slow. Isn't it something like 15 seconds? We don't always allow software power down in the plugin (for example, otherwise "Down" would shut down iPods in plugins).

I'm not going to discuss this further. This is unrelated to this patch.

PS: You can actually power off from any plugin which has a menu. Any other plugins already exits on a short press. In both cases, holding the power off button leads to a software shutdown. What you're trying to achieve just complicates things and possibly leads to unexpected behavior.

How is this unrelated? It's a discussion of whether or not this patch should do something, and why.

Your reasoning for why it shouldn't is false, which was my point. Saying "I refuse to actually demonstrate the cases in which my reasoning is true" doesn't really help your case any.

So please, on what specific players does allowing long-use of cancel cause a problem?

I tested the patch and noticed issue with holding direction keys.
for example, in robotfindskitten, if I hold left button, robot used to keep moving left.
but with the patch, robot moves only twice. (one is by PLA_LEFT and the other is by PLA_LEFT_REPEAT)

I do not see the reason to have PLA_QUIT_REPEAT. _QUIT seems tend to quit the plugin or enter it's menu. thus PLA_QUIT_REPEAT is not likely to be used imo… and as for gigabeat, holding power button ~3 sec would turn off the player.

I think it's useful to have PLA_MENU at least for the targets which has menu button.

Sync and rename PLA_START* to PLA_SELECT* and PLA_QUIT to PLA_CANCEL

teru: Adding more buttons is always on cost of the generic-ness, the ondio can't have a MENU, for example.
Scrollwheels are special, IMO. And I didn't make a generic button for the scrollwheel buttons, but special buttons only for those.

   rockbox-pla-rework-v1b.diff (56.5 KiB)

Fix crashes and problems with REPEAT.

   rockbox-pla-rework-v2.diff (63.4 KiB)

Remove the chopper hunk.

   rockbox-pla-rework-v2b.diff (62.6 KiB)

Few fixes to button mapping and to metronome (/me hopes mentronome is usable now).

   pla-rework-v3.diff (71.5 KiB)

worked well for me, using gigabeat X.
but i have issue with metronome.
i got following error when compiling.
> CC apps/plugins/metronome.c
> /home/teru/rockbox/apps/plugins/metronome.c:640: error: syntax error before ‘}’ token
> make: *** [/home/teru/rockbox/build/apps/plugins/metronome.o] Error 1
and messages　"start: press select" and "start: hold select" in plugin doesn't make much sense for me.
shouldn't they be "start: hold select" and "pause: press power" or something like?

Yea, depending on the target the text is bullshit. I'm thinking of having that text in the lib.

the text is wrong for all target, imo.
I tried to build simulaters for some keypad types and found issues in pluginlib_actions.c.
at line 127: CONFIG_LEYPAD → CONFIG_KEYPAD
at line 129: ( PLA_DOWN → { PLA_DOWN
at line 243: BUTTON_PREV → ?
BUTTON_PREV doesn't exist in H10.
at line 261: BUTTON_MINUS|BUTTON_MINUS → maybe BUTTON_MINUS|BUTTON_REPEAT ?
direction is not defined for IAUDIO_M3_PAD.

Fixed the problems you mentioned.

   pla-rework-v4.diff (74.7 KiB)

fix pitch detector.

   pla_rework5.diff (77 KiB)

I tested your patch on different targets in the simulator and it seems to work well.
The changes in the file bins.pl doesn't belong to this patch, right?

uhm no, that was just for making the test builds (see test builds forum). Thanks for testing, which targets did you test?

I tested the targets e200, fuze, c200, ipod video, mrobe, giga_s and onda in the simulator. I haven't found any bug.
I make minor changes to the game bubbles and update the new game codebuster to the new layout of pluginlib_actions.

   pla_rework6.diff (76.9 KiB)

Sync, add alarmclock add PLA_SELECT_REL which only fires when the button is released. basically you either use PLA_SELECT or PLA_SELECT_REL (and PLA_SELECT_REPEAT). _REL makes it possible to map different actions than on PLA_SELECT_REPEAT, PLA_SELECT not.

   pla_rework7.diff (84.3 KiB)

Sync, add mpio hd200 and fix tapping in metronome.

   pla_rework8.diff (86 KiB)

Fix some compiler warnings.

   pla_rework8b.diff (85.9 KiB)

Fix maze repeat actions.

   pla_rework8c.diff (88.1 KiB)

Commit candidate

   0001-S-10387-Rework-pluginlib... (89.8 KiB)