Support and General Use > Theming and Appearance Customization

Sansa Clip+ - how to display battery as a percentage all the time?

(1/1)

peedoe:
I have used a E200 for several years with a basic WPS screen i tweaked to my liking.  It has since died and I picked up a clip+ so I could continue to use rockbox, but the screen is much smaller so I have been playing with all the available themes for one I liked.  Found one I liked, but it has 1 'feature/design' that I would like to change.

It uses a battery meter graphic while playing, but displays the percentage when paused, ff, rewind, etc.

I prefer the percentage display at all times, but can't figure out how to tweak the wps file to show it.  I think this is the right area to tweak, but now it displays garbled text instead of the graphic or percentage.  Any help would be appreciated.

Original:
# statusbar
%Vl(a,0,1,-,11,0)
%xd(Af)|%Vd(d)|%?mp<%Vd(c)%xd(Aa)|%xd(Ab)|%Vd(c)%xd(Ac)|%xd(Ad)|%xd(Ae)>%?ps<%xd(S)>
%?bp<%xd(Ia)|%?bl<%xd(Ib)|%xd(Ic)|%xd(Id)|%xd(Ie)|%xd(If)>>

My version:
# statusbar
%Vl(a,0,1,-,11,0)
%xd(Af)|%Vd(d)|%?mp<%Vd(c)%xd(Aa)|%xd(Ab)|%Vd(c)%xd(Ac)|%xd(Ad)|%xd(Ae)>%?ps<%xd(S)>
%al%bl%%

I don't fully understand how the conditionals, viewports, and graphics work as my E200 theme didn't use any of that and was just a line by line set of text variables I wanted displayed.

TIA,
Peedoe

herefornow:
It would help if you could post the name/link to your favorite theme.

h

peedoe:
Sorry about that, I am using Simplest as the starting point.

http://themes.rockbox.org/index.php?themeid=1334&target=sansaclipplus

Peedoe

[Saint]:
Ok, Peedoe...here we go:
(promise me you'll ask questions if I lose you somewhere...)

First things first, I feel I should note that this theme has some pretty ugly coding practices, some are subjective (based on my personal belief of "correct" syntax structure), and some are downright wrong (possibly still subjective).

 - Calling conditional viewports that are displayed unconditionally.

The most obvious, and the first, example of this starts at line 18, where viewport label "c" is called without conditions, and is repeated on line 23 where viewport label "d" is called without conditions.
There's nothing wrong with doing this, in theory (that doesn't make it "right", though), but it adds needless complication to the theme.
The right way to do this would be to display the viewport unconditionally, without the unnecessary preloading. Because this viewport is always displayed, we can just set it as a static viewport. We can't do this in this theme though, at least not while keeping the intended functionality. The "c" viewport really should be split out into seperate viewports for the static and dynamic elements.


 - Using characters you know will be stripped from display to seperate code segments needlessly

This made me cringe. It is terribly hard to read, and it abuses the fact that the "|" char is a special character in the skin engine (see line 23), it is used to seperate segments of an n-tuple list of conditions. And if it is to be displayed as text, it needs to be escaped with a "%" character, as in "%|". The way it is used above, it will simply be stripped out by the parser and has the added downside of making the code harder to read.


 - Viewport collisions

If I read it right, the battery percentile display code just draws right overtop of the battery icon...which is pretty bad. The "right" thing to do here would be to gaurd against this case with appropriate conditions. For example, the battery percentile display code is only called in the while playing screen in two conditions:

1 when the device is stopped (this condition is irrelevant, as the while playing screen will be exited when playback stops, so this case will never be true)
2 when the device is paused

So, to do this the "right way", one would only display the battery icon in the (relevant) cases when the battery percentile isn't being displayed (so...ffwd; rwd; next; prev; play; but not stop, for the reason explained above).


 - Setting up conditions for cases that will never be true, or failing to account for cases that can be true.

I covered this briefly above with the battery percentile display code being called in the while playing screen in the "stop" case (line 23). There is another instance I dislike where the battery code checks if a charger is connected to the device, but fails to check if the device is actually charging or not which I think is useful information for the user (line 24). This isn't a very big issue, but it is one I personally dislike.
The final instance of this is where the theme checks for the presence of the "%album%" metadata tag, and then attempts to display the first level directory the audio is contained in if the check for "%album%" fails (line 45). This could fail if, for example, the audio wasn't contained in the root level of the device and the reason it bugs me is the fact that it would be very simple to check if a directory was actually present before trying to display the directory name (which may or may not even exist) meaning that if the current audio has no metadata, and is contained in the root of the device, the check will fail completely and no information will be displayed at all.


And these are just things I noticed from a very brief look. Looking further I noticed what one of the few conditions that actually has valid checks and syntax does, and for the life of me...I simply couldn't understand why the descision was made. For example:

 - (line 19)
This is just weird, this line checks if the volume is currently being changed, and if so, displays the battery icon. My assumption is that this is an error, ...but I really can't tell. If this were the only example of weirdness I have encountered in this theme, I would say it was definitely an error, but a part of me thinks it may be deliberate.

 - (line 23)
I mentioned this line earlier, but when I look into it deeper, it is doing some very weird things. For example, the very first thing this line does is display a "hold" icon unconditionally (which later gets drawn over), without checking the state of hold on the device at all. And secondarily it displays the volume in decibels, again unconditionally...I rather think that this would be much better suited for display when the volume is being changed, but the author seemed to think that displaying the battery status during volume change would be more appropriate.

 - (line 27)
I might just be being picky here, but this seems weird to me. I don't see the need to check the current config setting in this fashion when there is a simple skin tag that perform the same function.


There isn't an "easy" way to do what you want to do without completely disassembling the theme and starting from scratch. I'm happy to guide you through doing so if time allows me to, and I believe this is the best approach, while on the surface it may look like this theme "just works" there is some ugly and/or plain wrong code in there which I wouldn't want you learning from...these mistakes should be corrected, not repeated :)

Not all of the code needs to be scrapped, but, most of it should be. The items displayed in the statusbar should each have their own viewport for example, instead of each element being aligned in a larger viewport...as this makes it much more difficult to display the elements conditionally, currently the theme relies on draw order and simply drawing overtop of these existing elements instead of swapping them out conditionally. The fact that it works to begin with is just a happy accident caused by the order in which the elements are called.

I am sure my gargantuan post has left you with many questions, so instead of continuing now I'll pause this novel and let you gether these thoughts and come back to me. I hope I haven't scared you off, it really isn't as difficult as it sounds, but it can be difficult to "Do It Right (TM)".


[Saint]

Navigation

[0] Message Index