Rockbox

  • Status Assigned
  • Percent Complete
    0%
  • Task Type Patches
  • Category User Interface
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.2
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by kugel. - 2009-06-12
Last edited by kugel. - 2009-06-12

FS#10322 - Abortable splash

This adds asplash and asplashf (and vasplashf for internal use) which normal splashes but can be aborted with button presses.

Additionally, they return the action received (for CONTEXT_STD) so that the caller may handle aborts differently depending on the button.

Also I converted a few annoying splashes to use it (when loading themes, wps, fonts etc and the resume failed splash)

you seem to be overengineering….

my plan was to have ALL splashes either have 0 timeout, or a constant 3s (or something). There is no reason (and probably a bad idea) to have the splash return its value (although it should call the default_event_handler() for SYS_ events.

also, that first hunk looks like it shold be done seperatly of the rest of the patch.

- "you seem to be overengineering…."
Why?

- "my plan was to have ALL splashes either have 0 timeout, or a constant 3s (or something)"
It seems that your and my plans don't match

- "There is no reason (and probably a bad idea) to have the splash return its value"
Why? It could be used to ask the user a question or to only remove the splash if a specific button was pressed. does it hurt anywhere?

- "(although it should call the default_event_handler() for SYS_ events."
I will look into that.

- "also, that first hunk looks like it shold be done seperatly of the rest of the patch."
Yes, probably.

hey Tom and all,

Just my two cents. i feel that this particular patch will cause many problems for people who are not sighted. I feel that this patch would be best not applied at this time. I apologize but Tom's patch would be better where there is no place to have a question or something. Remember for us blind users everything you patch like this will then need to be spoken. i apologize if i sound rude but I am just making a point.

Scott, dont worry, There will be no change to blind users… (if Thomas does it properly…) the only tihng in the splash which will change is the time it is shown for. Actually, if every splash has a consistant timeout I would tihnk that would make it better for blind users. (Also, I imagine that having a beep when it exits is a good idea for you also)

The splashes should be spoken before you're even able to cancel, I theoretically see no problem here. Have you tested the patch, did you notice problems with voicing?

I think there are some situations you want a splash not to be cancelled (for example, the "Cancelled" one itself or the "Please reboot to enable", or some which report failure).

what is the logic behind not allowing thoe splashes to be cancelled? The whole point is that if you know what the splash says and are expecting it, why should you need to wait?

The point that not every splash is expected maybe? Especially for new users, some splashes are vital to be read.

then its the users fault if they cancel it before reading it.

I definitely disagree. You can cancel it accidentally quite easily.

You know what would also help too is to have at the beginning Rockbox say Rockbox started and at the end Rockbox stopped. I could probably come up with some wav files like that also. also consistent time outs yep they would be good with a beep or something like Rockbox is ready.

Thomas no i haven't tried it yet. I am kind of waiting for you guys to get all the patches for the Clip to be a little more stable. I would like to at least see that the boot loader and the main program be put in a beta package like you do the auto configurations. Then have that updated each day through a script.

This is the how it should be done

Admin
fg commented on 2009-06-16 22:12

Unless I'm reading things wrong, both patches here throw away events in at least some cases. That will lead to more bugs like FS#9957.

I don't quite understand. How does it throw events away?

It could probably check for SYS_EVENTS if you mean then (my patch aborts on any event, so that's no problem).

Admin
fg commented on 2009-06-17 08:23

Basically, whenever you read the button queue (which get_action() does), you have to check for SYS_EVENTs and handle them. Neither patch does this currently. Calling default_event_handler() is probably sufficient, although I'm not sure if that's safe in all places where splashes are used.

Here's an attempt to dismiss based purely on a button press, without eating any events…only tested on sim and no idea if it'll work for touchscreen targets.

Admin
fg commented on 2009-06-17 08:48

That will dismiss the splash when *any* event comes in, some of which are not triggered by the user (e.g. SYS_BATTERY_UPDATE or SYS_IAP_PERIODIC)

Huh, those also add to the button queue? That's not what I'd expect… ;-)

OK, this one ignores SYS_ events - it should probably also ignore scrollwheel events, but I don't have a scrollwheel target.

Ignoring SYS_ events is what Jonathan's patch also did (get_action() returns SYS_* when one comes in, which is ingored due to "== ACTION_STD_CANCEL"). I'm not entirely sure what your last patch tries to achieve.

Also, apps/ code really shoudn't use firmware/ functions in that way IMO.

I'm not very familiar with this area, but IIUC, Jonathan's patch actually read the queue. I want all events (including the SYS_* ones ) to wait in queue until they can be processed.

This patch is just to see if the UI/idea is sound - I'll wrap the test into buttton.c/.h for the real thing.

I think the SYS_ event should/could be handled in get_action() itself (i.e. calling default_event_handler() there, maybe returning something special in such a case.

well, its a general problem and I'm starting to think that those SYS_ events should go in a seperate queue

Either way, get_action would remove the button press, no?

I can't really see how it would be desirable to not eat the button. It seems illogical.

On the desktop you also need to click away dialog boxes, so I would actually expect that a button press is needed to get rid of the splash only.

Read the ML thread and IRC for details.

Advantages:
(1) You can abort a splash without waiting..
(2) There is no timing issue - especially useful if you can't see the screen. (The alternative option of using the cancel key might result in the underlying screen being cancelled if you press the key just as the splash times out, for example.)
(3) Corollary of (2) - it allows use of a timeout for those users who don't realise that splashes can be aborted with a keypress.
(4) It works without any keymap implications - no need to restrict to a particular button.
(5) The code changes are simple/limited.
(6) The voice and visual UI are consistent

Disadvantages:
(1) Users might not realise that the keypress will be used. - but I think they'd get used to it
(2) Users might dismiss a splash too easily - in which case, forget this change completely.

Feel free to add more and argue it out.

fml2 commented on 2009-06-20 16:57

Just my two cents: I like the idea of splashes that disappear when a button is pressed – and that button press should be processed as if there were no splash at all. Just think of splashes as modeless dialogs (this to reply to Thomas' "On the desktop you also need to click away dialog boxes")

Hi Scott, just to respond to your point made earlier in this task
"You know what would also help too is to have at the beginning Rockbox say Rockbox started and at the end Rockbox stopped. I could probably come up with
some wav files like that also. also consistent time outs yep they would be good with a beep or something like Rockbox is ready."

I don't think we really need the voice saying rockbox is ready or rockbox is stopped. This is because when we switch off the player we already get the voice saying shutting down, which to my mind you couldn't get any clearer than that.
When we start the player if a person has a voice you already get the first option in the menu announced that being files, or if you set rockbox to start in the file browser you get your first folder spelled out or announced.
I think your above ideas would just be overkill.

Can you give me an example though when a beep would be useful? I have never had any problems as far as I know with missing splashes or ccancelling them accidentally.

I am a blind user just to let you know.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing