Skip to content

screensaver: Add dynamic battery symbol to sleep screen message#12548

Merged
Frenzie merged 4 commits into
koreader:masterfrom
LostishCode:screensaver-battery
Nov 25, 2024
Merged

screensaver: Add dynamic battery symbol to sleep screen message#12548
Frenzie merged 4 commits into
koreader:masterfrom
LostishCode:screensaver-battery

Conversation

@LostishCode
Copy link
Copy Markdown
Contributor

@LostishCode LostishCode commented Sep 23, 2024

I have enjoyed (possibly over) customizing my sleep screen message. However I kept getting confused between the book completed percentage and the battery percentage. At first I just tried adding the battery symbol, but when looking into it, it was simpler to just reference the function that returns the dynamic battery icon.

I added the battery symbol to the prompt as %B:
sleep_screen_message_prompt

And examples of what it looks like (along with some other fun symbols I found along the way):
battery_64_percent
battery_89_percent
battery_89_percent_charging
battery_100_percent_charged

This was tested on a Kobo Clara 2E, but since it uses the same code that niluje wrote in frontend/ui/widget/touchmenu.lua for the touch menu icon, my guess is it will be device agnostic.


This change is Reviewable

- %B was added to frontend/ui/screensaver.lua to reference the battery symbol
  generated by powerd
- The default English screensaver help message was updated informing users
  of the addition
Comment thread frontend/ui/screensaver.lua Outdated
local powerd = Device:getPowerDevice()
if Device:hasAuxBattery() and powerd:isAuxBatteryConnected() then
batt_lvl = powerd:getCapacity() + powerd:getAuxCapacity()
batt_symbol = powerd:getBatterySymbol(powerd:isAuxCharged(), powerd:isAuxCharging(), aux_batt_lvl)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit too enthusiastic. You should also add a local batt_symbol = something up above where the same is done for batt_lvl.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I corrected the problem and the linter no longer throws errors. I also made a change so the icon acts the same as the footer as explained in the comment below. Though @NiLuJe said he had tentative concerns over the feature and I haven't heard back from him.

@Frenzie Frenzie added this to the 2024.09 milestone Sep 23, 2024
- batt_symbol is now correctly set as local variable
- fixed incorrectly used aux_batt_lvl where batt_lvl should have been used
@NiLuJe
Copy link
Copy Markdown
Member

NiLuJe commented Sep 23, 2024

I'm mildly wary of adding any kind of battery gauge in there... because it will be static. When the device is alseep, well, it's asleep.

As such, whatever will be shown on the message will be a snapshot taken at suspend time.

Which, granted, can be an interesting data point, but I fear may lead to confusion for a large section of users.

@LostishCode
Copy link
Copy Markdown
Contributor Author

I can see how a static battery symbol can be confusing some of the time. For example, if you start charging, put the device to sleep, then unplug the charger, the symbol doesn't change. On my device I didn't like not-seeing the battery_percentage_filled level so added a work around using the battery_charging_percent glyphs from \u{E784} to \u{E78A} that add a lightning bolt next to the battery_percentage_filled icon. The problem is there are only 7 of those glyphs in symbols.ttf (as nerdfonts appears to have since added the remaining 4) making the battery percentages have to glom to the nearest value.

In the case of not-charging though, I think the dynamic symbol is helpful. We already have the displayed percentage from %b, and the icons are just reflecting that value.

If the glyph being replaced by the battery-charging symbol is too inconsistent, we should be able to tell powerd:getBatterySymbol() that the battery always isn't charging-or-charged which would would then always return only the battery_percentage_filled icons.

@deepakonwww
Copy link
Copy Markdown

@LostishCode Which font does have this book icon and the nice-looking square brackets? I tried with my available fonts but was not able to type these characters. Could you please help me by providing the details of this font? Thanks in advance.

@LostishCode
Copy link
Copy Markdown
Contributor Author

@deepakonwww
The brackets are 【 】 which render here because they're normal unicode characters. The easiest way to add them is plug-in your device and edit the line ["screensaver_message"] = "your message here" in the file /path-to-your-device/.adds/koreader/settings.reader.lua. If you google "unicode brackets" you can find more examples of fancy brackets that should also all work.

- when an aux battery is present the aux battery will now act the same way
  as the footer icon.
@LostishCode
Copy link
Copy Markdown
Contributor Author

I took another look at how battery symbols are handled other places and it appears aux_batt_lvl gets treated differently depending on where it is. In frontend/apps/reader/modules/readerfooter.lua batt_lvl is averaged when an aux battery is present, in frontend/ui/widget/touchmenu.lua it's shown as two separate icons, and here %b will show devices with an aux battery as having a max capacity of 200% (strange, but surprisingly accurate).

The sleep screen icon needs to be consistent with other icons presented to the user, that means either the touchmenu icon or the footer icon, and I believe the footer icon makes more sense. The touchmenu battery splits into two icons when an aux battery is added which I don't think works well for the sleep screen; given it's a user controlled symbol, people might format around it and having it occasionally act as 2 icons feels inconsistent. The footer icon is also more likely to be the last icon users saw before sleeping as it's unlikely someone had the menu open before sleeping.

Though, this specific issue only applies when an aux battery is used. For all other use cases the battery icon acts the same across all three locations.

Personally I feel when there is one icon representing both a main and auxiliary battery the battery should be show cumulatively rather than averaged, but that's a separate issue that ought to be handled in a different thread.

@NiLuJe
Copy link
Copy Markdown
Member

NiLuJe commented Oct 6, 2024

The footer behavior was tweaked specifically because of the space constraints of that context, FWIW.

Note that the only device with an aux battery is the Sage with a PowerCover, and the PowerCover sucks really really really badly, so I wouldn't necessarily lose sleep over that ;).

That said, I do agree that the footer behavior is the one that makes more sense here.

(As for the average, that was necessary to be able to use the %-specific icons without jumping through extra hoops, IIRC)

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Nov 3, 2024

@NiLuJe Good for you?

Copy link
Copy Markdown
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting nit, but as far as the feature goes, it's perfectly harmless ;).

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Frenzie and @LostishCode)


frontend/ui/screensaver.lua line 197 at r3 (raw file):

        if Device:hasAuxBattery() and powerd:isAuxBatteryConnected() then
            batt_lvl = powerd:getCapacity() + powerd:getAuxCapacity()
           batt_symbol = powerd:getBatterySymbol(powerd:isAuxCharged(), powerd:isAuxCharging(), batt_lvl / 2)

Indentation mishap here?


frontend/ui/screensaver.lua line 200 at r3 (raw file):

        else
            batt_lvl = powerd:getCapacity()
           batt_symbol = powerd:getBatterySymbol(powerd:isCharged(), powerd:isCharging(), batt_lvl)

Ditto ;).

@Frenzie Frenzie modified the milestones: 2024.11, 2024.12 Nov 3, 2024
@NiLuJe NiLuJe requested a review from Frenzie November 25, 2024 19:13
Copy link
Copy Markdown
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Frenzie and @LostishCode)

@Frenzie Frenzie merged commit f5c6b56 into koreader:master Nov 25, 2024
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
…ader#12548)

- %B was added to frontend/ui/screensaver.lua to reference the battery symbol
  generated by powerd
- The default English screensaver help message was updated informing users
  of the addition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants