Rockbox

Tasklist

FS#7910 - Speak battery levels automatically

Attached to Project: Rockbox
Opened by Daniel Dalton (ddalton) - Tuesday, 09 October 2007, 10:39 GMT
Last edited by Stephane Doyon (sdoyon) - Saturday, 03 November 2007, 05:01 GMT
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This patch (A work in progress) will soon speak the battery level automatically at:
50%, 20% and 5%.

Also it says "charger inserted" and "Charger removed". Still needs further work for battery status notifications.
See the next few comments for the patches.
This task depends upon

Closed by  Stephane Doyon (sdoyon)
Saturday, 03 November 2007, 05:01 GMT
Reason for closing:  Accepted
Comment by Steve Bavin (pondlife) - Tuesday, 09 October 2007, 10:41 GMT
Try this one - compiles, but not tested at all (I don't have the voice file handy).
Comment by Steve Bavin (pondlife) - Tuesday, 09 October 2007, 11:07 GMT
Hi Daniel, a slightly smaller send_level(), using an array. Again, no idea if it works, please test.

static void send_level(void)
{
int current_level = battery_level();
const int levels[] = { 5, 20, 50, 0 };
const int *level = levels;
while (*level)
{
if (current_level <= *level && last_sent_level > *level)
{
last_sent_level = *level;
queue_broadcast(SYS_BATTERY_UPDATE, last_sent_level);
break;
}
level++;
}
}
Comment by Daniel Dalton (ddalton) - Tuesday, 09 October 2007, 11:51 GMT
Hi Steve,

Thanks for your help.
What exactly does your last patch do? I was using your older one so haven't got your most recent changes in this patch.

This patch has a setting to control weather the battery notifications should be spoken. It is under the voice menu.

Thanks to LinusN and pondlife for helping me out with this. I wouldn't have been able to do this with out your help so thanks.

Steve Is it ok that your most recent changes aren't here? Or should i add them tomorrow.
Thanks.
Comment by Daniel Dalton (ddalton) - Tuesday, 09 October 2007, 11:53 GMT
From the patch I was copying to learn how to make a menu I added the file type setting by mistake. I will fix it tomorrow.
Comment by Steve Bavin (pondlife) - Tuesday, 09 October 2007, 14:06 GMT
Hi Daniel,

The replacement routine above is slightly smaller, and allows the percentages when warnings occur to be customised easily (albeit by recompiling).

Comment by Daniel Dalton (ddalton) - Tuesday, 09 October 2007, 22:11 GMT
Hi Steve,

Ok fixed the setting.

Not yet tested on the sim so I have to do that later.

Also Steve's update is in this patch.
Thanks.
Comment by Daniel Dalton (ddalton) - Tuesday, 09 October 2007, 22:14 GMT
A couple of things I forgot to say:

Could someone sighted test the patch for me? What do you see in the voice menu. Does the option say "Battery status indications"?

Also do people have problems browsing with voice and this patch? (.talk clips) Also I would be interested to know about spell and number.

Needs more testing to make sure it says the battery levels when they actually drop. I saw it talk when I booted and when I unplugged the charger.
So so far so good.
Thanks.
Comment by Daniel Dalton (ddalton) - Tuesday, 09 October 2007, 22:20 GMT
Add a blank line that should probably be there.
Comment by Steve Bavin (pondlife) - Wednesday, 10 October 2007, 06:20 GMT
Hi Daniel,

A couple of minor points:
- There should not be a question mark after "Battery status notifications" in LANG_BATTERY_STATUS. Maybe "Talk battery status" would be a better phrase.
- The setting variable battery_status would be better named talk_battery_status IMHO. (Same for the settings item structure and LANG_ I guess.)
Comment by Daniel Dalton (ddalton) - Thursday, 11 October 2007, 06:57 GMT
Hi Steve,

Thanks for your help.
I am currently studying my c book so hopefully I can implement a few more complicated things into rockbox. But thanks for the suggestions. When I get a chance I will deffinetly fix up the patch as you said to.
Thanks again.
Comment by Sofian Babai (sofianbabai) - Sunday, 14 October 2007, 01:10 GMT
hi daniel, really interesting work. will this one work on an iaudio x5?
Comment by Daniel Dalton (ddalton) - Sunday, 14 October 2007, 04:09 GMT
On 14/10/2007 11:10 AM, Rockbox wrote:
> hi daniel, really interesting work. will this one work on an iaudio x5?

It should. Can you try it out? When charging and you go to the info screen do you hear the battery level followed by "charging"?
Thanks.

Comment by Stephane Doyon (sdoyon) - Monday, 15 October 2007, 03:35 GMT
Here's a reworked version of the patch.

Changed the wording for the english.lang entries and fixed formatting.

Call the setting "announce_battery_level".

Added a bit of silence around the announcement. I think it might catch
the user's attention better as the volume drops when the silence gets
mixed in.

Use talk_force_enqueue_next() so that the user doesn't interrupt the
announcement without even noticing it.

Have the announcement conditional only on
global_settings.announce_battery_level, not on
talk_menus_enabled(). There's a slight chance for a sighted user to want
the battery announcement and not the talking menus. And seems to me it
makes no sense to say nothing if battery announcements are enabled but
not talking menus. Comments on this?

However I've had to add another condition to stop it from talking if
talk_disable_menus() was called, such as from the recording screen. (If
this becomes official then I'll probably want to rename that function.)

Are there other situations/contexts in which it must not spontaneously
talk?

More descriptive names in powermgmt.c.

Changed the warning levels to 15, 30, 50. I know I had said 5, but after
a bit of investigation, I believe the battery safe level is generally
somewhere around 10 (please correct me if I'm wrong). For the warning to
be useful, it must happen a bit before disk access becomes
unsafe. I hope this doesn't have to be player-specific.

I think I like the way it announces the battery level on boot if it's
below 50%. Somehow I hadn't envisaged it that way, but I think I like it.

When I boot my x5 with the charger plugged in, it tells me so every time,
which is a bit annoying: boot, "Recent bookmarks", "Charger
inserted". Not sure I like that so much...

The charger notifications are really pretty orthogonal to the battery
level announcement.

I wasn't entirely sure the charger announcement were very useful, but now
that I bought a wall and car charger for my Sansa, I think I want them.

BTW Daniel thanks for your work on this!

To test this in real life and see if it interferes with stuff, I'll
use a version which announces levels say every 10%. I imagine it'll
be annoying, but I want it to pop up in different circumstances and
see how it affects things in "real" listening/use conditions.
Comment by Jonathan Gordon (jdgordon) - Monday, 15 October 2007, 05:16 GMT
I still think this should be rejected.. but anyway...

if your adding new variables into the gloabl settings struct you have to bump the plugin api min versino (plugin.h)
Comment by Daniel Dalton (ddalton) - Monday, 15 October 2007, 05:59 GMT
Hi Stephane,
I am glad you think it is useful.

When booting my iriver h300 I need to boot and then once rockbox is loaded plug my charger in.
Otherwise if you just plug the charger in before you boot it boots the OF.

I might do the same to make it speak every 10% to test it out.

I saw that you have it voice "charger inserted" and "charger removed" when anounce battery levels is off.
Do you think some people might find that annoying? I don't.
Thanks for trying it out and thanks for your update.
Comment by Daniel Dalton (ddalton) - Monday, 15 October 2007, 06:01 GMT
JdGordon
why shouldn't it be excepted?
Comment by Steve Bavin (pondlife) - Monday, 15 October 2007, 06:35 GMT
A couple of comments/questions:

"Call the setting "announce_battery_level" - I'd prefer talk_battery_level, we already use the terms talk and voice, why introduce announce as well?
"Use talk_force_enqueue_next() so that the user doesn't interrupt the announcement without even noticing it." - Does this prevent the user from interrupting the announcement should they wish to?

Regarding the test for talk_menus_enabled(), that should definitely be covered for recording and places where IRAM is in use (e.g. mpegplayer). I think I'd prefer the simplicity of just checking talk_menus_enabled() && global_settings.talk_battery_level. Alternatively, you could scrap the talk_menus_enabled() function completely, make menus check the settings directly and then internally prevent any talking whilst talk_disable_menus() has been called... and rename talk_disable_menus() to talk_disable() etc..
Comment by Stephane Doyon (sdoyon) - Wednesday, 17 October 2007, 02:41 GMT
Comment by Jonathan Gordon (jdgordon) - Monday, 15 October 2007, 05:16 GMT
>I still think this should be rejected.. but anyway...

I read a piece of IRC log where you said you think people should just
listen to their player until the battery dies and then forget about it
:-). Admittedly that's probably a frequent usage pattern, but me I try to
avoid it. You said you never check your player's battery level, but me I
check it frequently. I usually don't charge my player every night as the
level often doesn't go down that much between sessions of work on
Rockbox, and it's an extra effort (as opposed to having my player packed
and ready for the next morning. The 50% battery level warning should be
useful as a reminder: "Hey buddy, you've been using this thing more than
you think, don't forget to charge it tonight". I also think a warning a
bit before the level becomes unsafe could be useful. I have a kind of
phone book thing on my player, and sometimes short voice memos: I'll
usually want to save a bit of power to make sure I can get to these. I
use my player to listen to books more than music, and I like to plan to
have it charged and not run out, as it can be frustrating to have your
reading interrupted, and that can happen even at home. Being aware of the
battery level lets me do things like: charge over night, charge while
having supper so I'll have power to read afterwords, bring my sync cable
or car charger along on a trip, switch to my other player. Being able to
just glance at the statusbar to know your battery level must be
nice. Asking for it all the time is inconvenient, not enormously so I'll
grant you, but still it's annoying because 90% of the time for me the
level is above 60% and I wouldn't need to care. I think this is one of
the rare cases for voice where it can be nice to push info to the
user. Before I had Rockbox, I had programmed a rudimentary player and
note taker on a Linux-ified iPAQ handheld. Battery level announcements is
a feature I had implemented and I did find it useful in actual use. Plus
it seems to me this implementation into Rockbox is really not too
intrusive.

>if your adding new variables into the gloabl settings struct you have to
>bump the plugin api min versino (plugin.h)

Ah right, thanks.

Comment by Daniel Dalton (ddalton) - Monday, 15 October 2007, 05:59 GMT ?
>Otherwise if you just plug the charger in before you boot it boots the OF.

Same on Sansa, hmm except perhaps with a wall charger? X5 has a seperate
wall charger jack.

>I might do the same to make it speak every 10% to test it out.

You should :-).

>I saw that you have it voice "charger inserted" and "charger removed" when
>anounce battery levels is off.
>Do you think some people might find that annoying? I don't.

Well the charger messages might be slightly annoying. I agree that it's
really interesting to know when your plug isn't working; trouble is most
of the time, it states the obvious. If it could read your mind and tell
you when the power isn't working, we'd be golden :-). On another device I
worked on years ago, we used a sound effect instead of a message: upwards
beep on charger insert, downwards on disconnect. A brief and discreet
sound. It confirms your action, it's shorter, doesn't solicit your brain,
and doesn't talk over the current playback. I wonder if this technique
would be welcome?

As for associating the charger messages to the battery level announcement
setting: I don't see that the charger msgs are directly related to the
battery level announcements. The concept is indirectly related of course,
but it's not clear from the setting name, and the implementation is
independent. So I'm not convinced the charger msgs should go under that
setting. And the battery level announcements are spontaneous, while the
charger msgs are in response to a direct user action.

One thing I should do regardless is to report the charger state in the
Rockbox Info screen. Right now it only does it for #if CONFIG_CHARGING >=
CHARGING_MONITOR and not for CHARGING_SIMPLE.

Comment by Steve Bavin (pondlife) - Monday, 15 October 2007, 06:35 GMT ?
>"Call the setting "announce_battery_level" - I'd prefer talk_battery_level,
>we already use the terms talk and voice, why introduce announce as well?

Well I started with the english.lang entries first, and I thought to use
the word "Announce" to underline the spontaneous nature of this feature;
I thought that was an important distinction to make. And then I just used
the same thing for the setting name. I suppose it would make sense to
name the setting struct member "talk_", but I think announce is
descriptive and should stay in the option text. Good?

>"Use talk_force_enqueue_next() so that the user doesn't interrupt the
>announcement without even noticing it." - Does this prevent the user from
>interrupting the announcement should they wish to?

It does. But that's not a big deal IMO. This being spontaneous, the voice
probably has said "battery level", or at least "battery", before your
brain has caught on to what this is about. The message is about 1.5s in
total. Once you've heard that much, might as well get the number.

Seems to me there are two cases:

1) you're listening to music (playback) when this happens. There are no
means to interrupt the message, other than having it speak something else
which would take even longer.

2) You're in a menu or some other context in which your probably pressing
buttons more often, generally causing something to be said on each
press. If we let any new voice utterance interrupt the battery message,
it's likely you'll press a button as the battery message is enqueued and
then you'll just miss it, or stop it half way through
unintentionally. The only way I see to make sure the interruption is
intentional is to mark time and allow it only if we have say the first
0.5s of the message already spoken. But I don't think the complexity and
extra code would be justified.

>Regarding the test for talk_menus_enabled(), that should definitely be
>covered for recording and places where IRAM is in use (e.g. mpegplayer). I

mpegplayer now also calls talk_disable_menus(), so that's covered.

>think I'd prefer the simplicity of just checking talk_menus_enabled() &&

but that makes it depend on talking menus being enabled in the settings.
All the other talk options are independent.

>global_settings.talk_battery_level. Alternatively, you could scrap the
>talk_menus_enabled() function completely, make menus check the settings
>directly and then internally prevent any talking whilst talk_disable_menus()
>has been called... and rename talk_disable_menus() to talk_disable() etc..

Well what the callers really mean is to prevent any sort of talking at
all (to me that's sad really, but oh well...), so I thought I'd rename
the functions to talk_disable(). And I could test that when talking is
disabled, we don't talk through talk_spell or talk_file or any other way.
Comment by Stephane Doyon (sdoyon) - Saturday, 20 October 2007, 05:05 GMT
pondlife: thanks for the talk disabling cleanup. (Even if it took me
a while to re-base all my patches :-) ).

Here's an updated patch.
Comment by Daniel Dalton (ddalton) - Saturday, 20 October 2007, 05:40 GMT
Thanks sdoyon for the update. Can I update this one because I am the author of the patch (sort of). Or do you want to maintain it yourself?
Thanks again for all the help I got with this.
Comment by Daniel Dalton (ddalton) - Sunday, 21 October 2007, 02:41 GMT
Remove the voicing of a charger. Now it beeps. That is in
FS#8006
Here is the updated patch.
Comment by harry tu (bookshare) - Sunday, 21 October 2007, 16:02 GMT
I cannot apply it. Here is the reg file.
Comment by harry tu (bookshare) - Sunday, 21 October 2007, 16:15 GMT
My mistake, cancel that

Loading...