Rockbox

Tasklist

FS#10853 - radio screen skin support

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Monday, 14 December 2009, 07:51 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 12 May 2010, 10:38 GMT
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Full skin support for thr radio screen!

new tokens: (token letters can be changed if someone comes up with a better idea)
tuner present: %tp
tuner "tuned": %tt
scan mode: %tm<scan|preset>
minimum frequency: %ta
maximum frequency: %tb
current frequency: %tf
preset number: %TiX (where X is a number, 0 is current, -1 is previous preset, 1 is next... numbers have no limits but must be there!)
preset name %TnX
preset freq: %TfX
preset count: %Tc (i.e number of presets)
target has RDS support: %tx
RDS name: %ty
RDS text: %tz

and new recording tags for HWCODEC which records from inside the fm screen
%Rr - is actually recording? (play status will be showing fm always so this is needed)
%Rs - seconds recorded
%Rn - minutes recorded
%Rh - hours recorded.


To load a skin add the usual skin line to your .cfg "fms: /.rockbox/wps/name.fms"
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Wednesday, 12 May 2010, 10:38 GMT
Reason for closing:  Accepted
Additional comments about closing:  finally in svn! r25964
Comment by Tomer Shalev (tomers) - Monday, 14 December 2009, 08:00 GMT
Good work!
Can we used some frequency scale for the radio? Even a progress bar that ranges from minimum to maximum frequency will do.
Comment by Jonathan Gordon (jdgordon) - Monday, 14 December 2009, 08:06 GMT
funny you should mention that :p
%pb (progressbar) is now working.
Comment by Jonathan Gordon (jdgordon) - Sunday, 31 January 2010, 06:03 GMT
sync and update. I apparently didnt update the last time I played with this.
Comment by Jonathan Gordon (jdgordon) - Sunday, 31 January 2010, 08:26 GMT
and add the last of the missing tags.. namely RDS and fm recording support.

someone with a archos recorderFM needs to test this patch. This is I tihnk ready for commit
Comment by Alex Parker (BigBambi) - Sunday, 31 January 2010, 22:19 GMT
Excellent stuff! I think I've found a couple of bugs however:

a) In preset mode if you skip through presets using left/right then the preset name %TnX doesn't update - but if you select the preset from the list then it works fine
b) In scan mode, the frequency doesn't show properly. If the current frequency is the same as a preset it shows that, but if not it shows 106.6
c) When you switch between preset and scan mode then whilst the mode switches, %tm doesn't update until you leave the screen and come back e.g. go to the menu and back

The fms/sbs I'm using is attached.
Comment by Alex Parker (BigBambi) - Sunday, 31 January 2010, 22:45 GMT
OK, ignore b), that was an error and c) might be - I need to check more :)

a) is real however
Comment by Jonathan Gordon (jdgordon) - Monday, 01 February 2010, 07:53 GMT
this should fix a and c
Comment by Mark Arigo (lowlight) - Wednesday, 03 February 2010, 16:59 GMT
jdgordon:
Would it be possible to display the current frequency using custom bitmap numbers? Perhaps tokenizing each digit of the current frequency and using a bitmap strip with the digits 0-9 and a decimal point. That would allow for a prominent frequency display (similar to most OF radio screens) without using multi-font to get larger digits. It would look good on large displays where the FM screen is mostly empty.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 03 February 2010, 17:13 GMT
it is possible, but its not going to happen. There is a patch on the tracker to do this but its been rejected.
Comment by Mark Arigo (lowlight) - Wednesday, 03 February 2010, 19:15 GMT
Rejected? That hasn't stopped you before ;) Think of it as album art for the FM screen...some will use it, others won't.

I thought that with the skin engine this would be easy (although I don't really know how the skin engine works). Wouldn't it just be 6 extra tokens (xxx.xx). I assume the bitmap loading and display is already in the engine. The bitmap strip would need digits 0-9, a decimal, and a black space (for the leading digit for frequency < 100). If it uses the skin buffer, there's no real additional cost.

Maybe I'm just misunderstanding how the skin engine & tokens work.
Comment by Jonathan Gordon (jdgordon) - Friday, 05 February 2010, 23:49 GMT
oh its a dead simple patch.  FS#9710 . I don't really remember why it was rejected, if you're game we can try bringing it back. especially now that the buffer usage is 0 if you dont use the tokens.
Comment by Jonathan Gordon (jdgordon) - Sunday, 07 February 2010, 17:57 GMT
fixed hwcodec compile
Comment by Jonathan Gordon (jdgordon) - Monday, 08 February 2010, 05:45 GMT
fix the warnings and the WPS not hiding the statusbar correctly. This is just about ready me thinks.
I want to nail down the tokens we want to support first though. is the current set good? or should they be fixed?

also, we need to deprecate LANG_FM_STATION and come up with a new label for it. Changing the source string is BAD! especially when its fed directly to printf
Comment by Jonathan Gordon (jdgordon) - Monday, 08 February 2010, 17:27 GMT
sync to svn
Comment by Jonathan Gordon (jdgordon) - Tuesday, 09 February 2010, 04:49 GMT
minor change, dont update the screen unless we need to (the whole fm screen is pretty damn static), but do a full update when it does change.

I want to commit this already, so testers neded
Comment by Alex Parker (BigBambi) - Tuesday, 09 February 2010, 20:30 GMT
Looks great, with two minor provisos:

The backdrop only gets drawn in viewports - the same issue as was fixed for the WPS in r24571

If the fms contains %C it breaks on target (but not on sim). I know %C isn't supported yet for fms, but JdGordon requested I pop it here :)
Comment by Jonathan Gordon (jdgordon) - Thursday, 11 February 2010, 07:29 GMT
how did %C break it on target? it looks like it is safe (although, I'm ok with the workaround of dont use that tag)
attached should fix the backdrop isssue. I'm commiting this tomorow night!
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 February 2010, 22:24 GMT
resync
Comment by Jonathan Gordon (jdgordon) - Tuesday, 16 February 2010, 06:15 GMT
remove the %Tf/n/iX buisness and just use %tf/n/i for the "current" preset. "current" being the current one, or the one in the playlist viewer.
I don't think we need to add special handling for the next one because the playlist viewer is a better way to do this?

no other changes in this version.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 16 February 2010, 20:22 GMT
bah
Comment by Jonathan Gordon (jdgordon) - Wednesday, 17 February 2010, 06:11 GMT
fixed backdrop, scrolling, added the file handling (i.e menu items)
Comment by Alex Parker (BigBambi) - Wednesday, 17 February 2010, 19:54 GMT
A couple of little things: The menu backdrop shows through on scrolling lines, and when you exit the fms to the menu the menu backdrop isn't properly drawn again.

dump1 attached shows a scrolling line in the WPS, and dump2 shows the menu immediately after leaving the fms

There is also a small glitch at the top of the fms where the menu backdrop shows through the section of the UI viewport there. This also happens on the WPS, but SVN is fine.

I can attach the theme if needed.
Comment by Alex Parker (BigBambi) - Wednesday, 17 February 2010, 19:55 GMT
Attached is a patch to make the playlist viewer support alignment tags with scrolling (by JdGordon). It is on top of the latest fms patch.
Comment by Jonathan Gordon (jdgordon) - Thursday, 18 February 2010, 06:19 GMT
sync and fix the backdrop issue.
can you upload a theme with the scrolling issue?
Comment by Jonathan Gordon (jdgordon) - Thursday, 18 February 2010, 06:21 GMT
oh, hmm.. looks like a bad sync a while back... this one should fix the backdrop issue
Comment by Alex Parker (BigBambi) - Thursday, 18 February 2010, 17:13 GMT
All looking good - I can't see any remaining issues with the testing I've done :)

Thanks :)
Comment by Jonathan Gordon (jdgordon) - Friday, 19 February 2010, 06:13 GMT
still a bit of code duplication that can be cleaned up, but this is the first version that I tihnk is ready for commit.
Comment by Jonathan Gordon (jdgordon) - Monday, 22 February 2010, 07:35 GMT
sync...
Comment by Jonathan Gordon (jdgordon) - Sunday, 28 February 2010, 21:43 GMT
sync
Comment by Jonathan Gordon (jdgordon) - Wednesday, 03 March 2010, 08:35 GMT
another sync...
There is a problem where there isnt enough room on the buffer for the fms on non colour targets, so if you test on mono/grey then modify line 58 of apps/gui/skin_engine/skin_buffer.c. change that 2 to some larger number (10 or 12 perhaps) and it should work.
Comment by Jonathan Gordon (jdgordon) - Friday, 12 March 2010, 01:34 GMT
sync
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 March 2010, 10:54 GMT
AlexP-proof the two preset tags (fixes the issue where it hangs if you dont have any presets loaded)
Comment by Michael Chicoine (mc2739) - Thursday, 18 March 2010, 02:06 GMT
I have found a couple of issues while testing the latest patch.

1. In the main menu, while FM Radio is highlighted, if you bring up the context menu and load a preset file you are taken into the FM screen after the file is loaded. If there is a progress bar in the FMS (including the in-built default), the progress bar does not work. The progress bar will work properly if you exit the radio screen and reenter.

2. In scan mode, I use this %?Tn<%s%ac%Tn|%Tf> to display a presets name (or frequency if it is unnamed) when a presets frequency is tuned. When not on a preset frequency, the preset name of the highest preset is displayed. I am attaching a fix for this, although it may not be the best way to handle this issue.
Comment by Jonathan Gordon (jdgordon) - Sunday, 09 May 2010, 12:21 GMT
straight sync from my previous patch.. starting numbers again.
Michael I dont quite understand your fix?
Comment by Michael Chicoine (mc2739) - Sunday, 09 May 2010, 19:17 GMT
Here is a better fix:

change skin_tokens.c line 390 from "if (radio_preset_count() == 0)" to "if (radio_preset_count() == 0 || preset == -1)"
Comment by Jonathan Gordon (jdgordon) - Monday, 10 May 2010, 10:36 GMT
Thanks, that change is in this patch, also fixed the issue in your comment 2 up about the bar not working when loading a presets file.

Loading...