Rockbox

Tasklist

FS#10494 - Ipod fm remote support for 4G,5G,color,Nano 1G

Attached to Project: Rockbox
Opened by Laurent Gautier (Creposucre) - Friday, 07 August 2009, 16:58 GMT
Last edited by Jonathan Gordon (jdgordon) - Tuesday, 12 January 2010, 23:04 GMT
Task Type Patches
Category FM Tuner
Status Closed
Assigned To No-one
Operating System iPod 5G
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 adds full support for the apple ipod fm remote. Targets 4G, 5G, color, Nano 1G
It features:

FM tuner with RDS text capabilities
volume control from remote

I expect (and hope) some docks and others remote tuners to work with it.
This is my first patch, so I think it's far from being perfect (RDS architecture, etc), just give me some clues ;)

Revision 22195
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Tuesday, 12 January 2010, 23:04 GMT
Reason for closing:  Fixed
Comment by Laurent Gautier (Creposucre) - Friday, 07 August 2009, 19:23 GMT
just another one...
N.B: the FM radio menu displays sounds settings, but it cannot have effects on the ipod line-in, which is used. It can have effects on the regular ipod headphone jack; the headphones are the antenna, though...
Comment by Thomas Martitz (kugel.) - Friday, 07 August 2009, 22:29 GMT
I think the RDS archtitecture should be somewhat generic, the gigabeat S also has RDS support (but we don't use it ATM), but that can also be done later IMO.
Comment by Nils Wallménius (nls) - Saturday, 08 August 2009, 11:06 GMT
First of all; Nice work!

I don't have any working ipod nor a fm remote so I can't test this personally but I have a couple of questions and remarks.
First some minor style things:
We have a set of style rules in docs/CONTRIBUTING if you need to reference them but basically indent with spaces (preferrably 4), no tabs, no // comments and all-lower-case identifiers (except for #defines and enums):
Also the "static bool powered = false;" in tuner.c seems unecessary

Now the real questions ;)
What is the meaning of the HAVE_RMT_SCAN_MODE define and is it necessary to introduce code specific to this in the generic radio.c rather than in the driver?

Is this the only fm remote available for ipods or do they all work the same way?
Have you tested how rockbox reacts if you unplug the remote while in the fm screen for example?
AFAIK all other targets either have the tuner present all the time or don't have it at all so this is a new situation that might uncover hidden assumptions.
Comment by Rosso Maltese (asettico) - Saturday, 08 August 2009, 11:22 GMT
It breaks the simulator build. Here there are the messages:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c: In function ‘recsource_func’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c:88: error: ‘LANG_FM_RADIO’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c:88: error: (Each undeclared identifier is reported only once
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c:88: error: for each function it appears in.)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c:88: warning: missing initializer
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c:88: warning: (near initialization for ‘names[2].string’)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/menus/recording_menu.c:94: warning: implicit declaration of function ‘radio_hardware_present’
Comment by Laurent Gautier (Creposucre) - Sunday, 09 August 2009, 10:19 GMT
nls: Thank you!

looks like i did all the "no do style"... I hopefully corrected everything in the attached patch

HAVE_RMT_SCAN_MODE define a scan mode that is done by another hardware (remote): in our case, we send a scan order and the remote responds with a frequency.
The reason why it was done like that is because the scan results are really precise (and faster). With the specific radio code (execute every frequency and looks at the rx power) there is a lot of bad frequency, even with a quite high RSSi threshold . Maybe i'm doing something wrong, I'm working on it.

I think there is another remote for ipod by Belkin, I don't know if it works with it since I couldn't test it.

Rockbox detects the tuner, but you have to go out and come back to the root_menu to refresh it. I am not sure how I sould do that for the moment.
If you unplug the remote while in FM screen, it goes out of it, asking to save any change.

The code added in default_event_handler seems to take care pretty that situation, but the root_menu redraw is the last thing
If anyone want to test both modes ("remote_scan_mode" and the regular one), you may just (un)comment #define HAVE_RMT_SCAN_MODE in ipod_remote_tuner.h

asettico: I made the correction, shouldn't break the sim anymore
Comment by Rosso Maltese (asettico) - Sunday, 09 August 2009, 11:38 GMT
I tried the new patch, there are still some simulator build errors, the FW builds ok.
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘radio_screen’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:571: error: ‘LANG_FM_FIRST_AUTOSCAN’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:571: error: (Each undeclared identifier is reported only once
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:571: error: for each function it appears in.)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:649: error: ‘LANG_FM_SAVE_CHANGES’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:999: error: ‘LANG_FM_STATION’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1010: error: ‘LANG_PRESET’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1011: error: ‘LANG_RADIO_SCAN_MODE’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1016: warning: implicit declaration of function ‘tuner_get_text’
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1016: warning: format ‘%s’ expects type ‘char *’, but argument 4 has type ‘int’
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1020: warning: format ‘%s’ expects type ‘char *’, but argument 4 has type ‘int’
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘radio_save_presets’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1175: error: ‘LANG_FM_PRESET_SAVE_FAILED’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘radio_add_preset’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1260: error: ‘LANG_FM_NO_FREE_PRESETS’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘save_preset_list’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1370: error: ‘LANG_FM_NO_PRESETS’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: At top level:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1391: error: ‘LANG_FM_EDIT_PRESET’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1394: error: ‘LANG_FM_DELETE_PRESET’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1405: error: ‘LANG_PRESET’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘presets_get_name’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1420: error: ‘LANG_FM_DEFAULT_PRESET_NAME’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘get_mode_text’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1528: error: ‘LANG_RADIO_SCAN_MODE’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: In function ‘scan_presets’:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1551: error: ‘LANG_FM_CLEAR_PRESETS’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1576: error: ‘LANG_FM_SCANNING’ undeclared (first use in this function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1545: warning: unused variable ‘old_freq’
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c: At top level:
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1686: error: ‘LANG_FM_ADD_PRESET’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1691: error: ‘LANG_FM_PRESET_LOAD’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1693: error: ‘LANG_FM_PRESET_SAVE’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1695: error: ‘LANG_FM_PRESET_CLEAR’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1697: error: ‘LANG_FM_SCAN_PRESETS’ undeclared here (not in a function)
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/recorder/radio.c:1701: error: ‘LANG_FM_MENU’ undeclared here (not in a function)
Comment by Laurent Gautier (Creposucre) - Sunday, 09 August 2009, 19:28 GMT
Ok, let's do that step by step:

I applied the attached patch on affected targets (normal and sim) and on non affected targets (3G)
Synced with on fresh build 22228, "make clean" each time, everything ok

I removed all the RDS stuff for the moment, I just let the two scanning possiblities (same thing, you may just (un)comment #define HAVE_RMT_SCAN_MODE in ipod_remote_tuner.h) and a display on the main FM screen for radio signal power.

The default mode (rockbox, scanning step by step) stop most of the time 4 times around a radio channel in Europe freq settings (50KHz steps)
The "remote scan mode" is ok, selecting one time only the radios, even with a low level signal.

Does it exist others tuner-in-a-remote for others rockboxed targets?

P.S: Does anyone owns a dock with tuner that is controllable by the ipod?
Comment by Jonathan Gordon (jdgordon) - Monday, 10 August 2009, 06:15 GMT
well first off, this patch makes my video (but not my mini2g) work with my alarm clock dock thing (power doesnt work still, but we are still much further than svn)..
I cant test the fm part of the patch, and cant really comment on the code other than is it possible to move more of the radio aditions in iap.c into firmware/ ?

edit: apparently iap isnt enabled for mini2g in the config... enabling to find out what happens
Comment by Laurent Gautier (Creposucre) - Monday, 10 August 2009, 07:55 GMT
I'm glad it's working for your dock!

I'll look into the code to try to move some others things into firmware.

About the mini2G, I am not sure that the serial is working on it, so no iap. The 4G, 5G, color and nano1G are the only ones to have serial working, unfortunately.

Can you tell me more about the power thing, what is it supposed to do?
Comment by Jonathan Gordon (jdgordon) - Monday, 10 August 2009, 16:10 GMT
pressing power on the dock turns the dock off (and should turn the ipod off) but because the ipod doesnt turn off, the dock imediatly turns on again and starts playing music (making it very hard to use as an alarm clock :) )
yeah, apparently the serial stuff isnt set up, although isnt the mini2g the same PP version as the nano?
Comment by Laurent Gautier (Creposucre) - Monday, 10 August 2009, 17:07 GMT
Okay, i'll look at that.

About the mini, looks like it is a PP5020 (same as color, supported). Nano is PP5021C (detected as a PP5022)

Mini has accessory supply, though. Would it be dangerous to enable serial on it, supposing it's the same hardware than color?

If not, you may just add #define IPOD_ACCESSORY_PROTOCOL, #define HAVE_SERIAL in your config file to have it working and add " || (MODEL_NUMBER == 11)" next to (MODEL_NUMBER == 8) in serial.c

Also, let me know what are your results with fs#9951 (iap logging) about the power command
Comment by Jonathan Gordon (jdgordon) - Monday, 10 August 2009, 17:17 GMT
I applied that patch after this one and didnt get any log file... I may have stuffed up the merge though...
I'll give it (and the serial.c suggestion) a go tonight and let you know
Comment by Laurent Gautier (Creposucre) - Monday, 10 August 2009, 17:32 GMT
if you hadn't any error compiling (i think it was missing a #define sprintf.h), your problem is maybe with the HD: when something is sent to the ipod, it tries to write on his disk ( which is most of the time in sleep mode) and has to wake him up. Since it takes quite a long time to spin up, the iap initialization is messed up and isn't recognized.

you may define in settings a "long HD time before sleep
Comment by Jonathan Gordon (jdgordon) - Monday, 10 August 2009, 17:41 GMT
how useful are the logs? it might be worthwhile doing it properly, bufalloc() a chunk of ram (12k sounds like a nice number, how big is each message usually?) if logging is enabled, write to that buffer and use the disk_idle_notify event to dump the cache to disk to get around these HDD issues
Comment by Laurent Gautier (Creposucre) - Monday, 10 August 2009, 18:49 GMT
The logs would be useful to see where it blocks, and thereby complete the "rockbox apple protocol" to enable non supported yet fonctionnality (like the power thing)

12k sounds nice, it should take a long time to be used. Most of the time, packets logged aren't bigger than 10 bytes

+1 for a proper log code :)
Comment by Jonathan Gordon (jdgordon) - Monday, 10 August 2009, 19:17 GMT
brought this up very quickly in IRC and instead it might be a really useful addition to the debug menu instead.. possibly with the ability to press a button and have the logs marked somehow (or flick hold which probably would have no affect on the accessory)
Comment by Laurent Gautier (Creposucre) - Thursday, 13 August 2009, 14:37 GMT
I finally managed to remove most of the code from generic radio.c. Only the code to handle disjunction for radio hardwares (like the remote) remains.

JdGordon: I added some code to handle power off button, can you test it? Also, I would like to redraw the root menu on a "radio_hardware_present" change, do you know how it can be done?

One other thing: when I access to the submenus in radio settings (radio region & mono mode) and i try to change the settings, it stucks the menu and loop the execution of that setting. Does anyone can test the behavior on an other tuner-ed target?
Or maybe it's a keymap problem, but I can't find it...
Comment by Gman (Thecoolgman) - Saturday, 15 August 2009, 05:40 GMT
Hi I just got my iPod FM Radio today so if I can get SVN to work I can test this out.
Comment by Laurent Gautier (Creposucre) - Saturday, 15 August 2009, 08:17 GMT
You bought it especially for that? Man, you put pressure on me :-)
Comment by Antoine Cellerier (dionoea) - Friday, 28 August 2009, 18:08 GMT
Hello. I just tested the latest version of you patch on an ipod video with the apple radio remote (Model A1187 according to the text under the clip). It compiles fine but I can't get any sound when trying to listen to the radio. Is there anything special to do to get it to work? or some debug info that I could provide which would help?
(btw, you have lots of tabs in your patch, you should replace those with spaces)
Comment by Laurent Gautier (Creposucre) - Saturday, 29 August 2009, 09:00 GMT
Hi Antoine,

there is nothing special to do, except press play if it's mute (main radio screen). I have the same model, it should works fine. The only thing is that to enter radio mode, you have to enter a sub menu and go back to root to refresh the screen and to be able to see the radio icon.

If you can't get it to work, you might add FS 9951 that will log the commands, I'll look at it .

About the tabs, I thought I'd removed everything, but I'll correct everything in the next patch (almost done)
Comment by Laurent Gautier (Creposucre) - Monday, 31 August 2009, 16:39 GMT
Ok, what's new on this one:

Everything relating to the tuner is in the driver file ipod_remote_tuner.c. Almost nothing is changed in the others files
The remote FF and RW buttons are now supported
Clean sub-menu selection, avoiding looping execution
synced on 22570

One other thing: does this matter if I swap "curr_freq = global_status.last_frequency * fmr->freq_step + fmr->freq_min" and "tuner_set(RADIO_SLEEP, 0);" ?
With that remote, we have to power on the tuner first before station tuning.

Antoine: Is it ok with your remote?
Comment by Antoine Cellerier (dionoea) - Wednesday, 02 September 2009, 21:52 GMT
Hum ... It's working fine now. I must have messed up something the first time I tried (the battery was really low, maybe that prevented a succesfull powerup of the remote?)
Volume control of the ipod by the remote also works fine.

About other accessories: I tested with the Apple dock (A1153) using the IR remote to control playback. The prev/play/next buttons work fine (as before). Your patch adds support for the volume up/down buttons. So the only missing button (or at least it's not bound to any action) is the menu button. I've tried having a look at the IAP PACKET value in the debug menu and it does seem to change when I click the menu button :/ Is there any way to dump more information?
Comment by Laurent Gautier (Creposucre) - Thursday, 03 September 2009, 07:58 GMT
Glad it's working for you too!

I'll have a look for the menu button to see what I can do. In the meanwhile, you might want to test FS 9951, you'll find a patch to log thoses informations in a file.

You should only pay attention to the fact that the infos are written only when the disk spin up for something else.
Comment by Rosso Maltese (asettico) - Friday, 04 September 2009, 13:46 GMT
Sync to r22617.

Excluded source dependencies for the boot loader. The error message was:
make: *** No rule to make target `/home/asettico-9.04/Sviluppo/rockbox/rockbox/build_bootloader/settings.h', needed by `/home/asettico-9.04/Sviluppo/rockbox/rockbox/build_bootloader/firmware/drivers/tuner/ipod_remote_tuner.o'. Stop.
Comment by Rosso Maltese (asettico) - Friday, 04 September 2009, 13:53 GMT
Sorry, forget the previous attach.
svn diff DO NOT consider new files!!!
Comment by Laurent Gautier (Creposucre) - Friday, 04 September 2009, 16:55 GMT
This one includes correction by asettico (bootloader thing) + hopefully adds support for a remote menu button (Antoine, if you can test this out)

It adds again support for RDS infos, it should compile and work... One thing I'm not sure about is with HAVE_TUNER_MULTI in tuner.c. Maybe somebody more at ease with this code can bring some light...
Comment by Rosso Maltese (asettico) - Saturday, 05 September 2009, 21:27 GMT
Just another fix about calling tuner_get_rds_info() in apps/recorder/radio.c when building for simulator.
Comment by Antoine Cellerier (dionoea) - Wednesday, 09 September 2009, 21:52 GMT
It looks like the menu button still doesn't work (at least I didn't notice any action when pressing it).

I applied the logging patch (without the fm radio one) to get a log of all the serial data. Actions executed were (in order):
Ipod fm remote plug (A1187), plus, minus, back, fwd, play, hold, unhold, unplug, Ipod dock plug (dock A1153 + IR remote A1156), plus, minus, back, fwd, play, menu, unplug.
All the key presses were short presses. I don't know if long presses behave differently.

Edit: I was expecting repeated patterns from the similar actions (like plus, minus, back and fwd) on both input methods but I can't spot it in the log file. Did something go wrong while logging?
Comment by Laurent Gautier (Creposucre) - Thursday, 10 September 2009, 12:05 GMT
This is because there is no button informations (with the A1187), it stops in the middle of its initialization, so you can't go further and use buttons in the OF way.

About the ipod dock, it seems that only the back fwd and play button codes are here. Is it complete or part of a bigger file?

Can you retest with the fm remote patch applied?
Comment by Laurent Gautier (Creposucre) - Monday, 28 September 2009, 16:45 GMT
Hi! Today: V10. Synced with 22847

I removed all the support for buttons that aren't on the fm remote (menu, power_off). I removed the config multi thing since we are not concerned here.

I updated the region settings too.

Give me your feedback whenever you get the chance to test it
Comment by Rosso Maltese (asettico) - Saturday, 17 October 2009, 11:07 GMT
The patch doesn't apply and I can't sync it because it's quite different by the original sources.
And what about the one with RDS?
TIA.
Comment by Laurent Gautier (Creposucre) - Saturday, 17 October 2009, 21:42 GMT
Synced with 23234.

I added a few things in the keymap regarding the remote.

This patch includes the RDS support.

Please report your feedback
Comment by Rosso Maltese (asettico) - Saturday, 24 October 2009, 17:54 GMT
It seems that the Apple FM remote receiver is not seen when plugged in the dock connector.
I'm forgetting to do something after plug the device?
Comment by Laurent Gautier (Creposucre) - Saturday, 24 October 2009, 21:25 GMT
You just need to refresh the menu in order to display the radio icon: you may enter a sub menu and go back to the main screen and you will be able to access the radio mode.

I haven't found yet a clean way to refresh the main menu when the remote is plugged in, that's why this thing is needed.
Comment by Laurent Gautier (Creposucre) - Wednesday, 11 November 2009, 18:52 GMT
Synced with 23610
Comment by Jonathan Gordon (jdgordon) - Thursday, 12 November 2009, 06:30 GMT
my ipod video is fubar apparently and I cant test this out... but I'm happy to commit it if you think its good to go.
Comment by Laurent Gautier (Creposucre) - Thursday, 12 November 2009, 21:22 GMT
Cool!

I made a single patch that includes 10623 (dock patch) and this one (FM remote with RDS), easily commitable
This patch shouldn't harm anything, I think it's good to go as it is.

Two things, quite minor:
- you still have to refresh the main menu if you connect the fm remote while in this menu
- you can't control the remote volume with the ipod yet

This can be done later IMHO

Your HDD crashed, or is it something else?

And did you tried some experimentations to enable the serial on you mini?
Comment by Jonathan Gordon (jdgordon) - Thursday, 12 November 2009, 21:40 GMT
yay! I'm not going crazy.... I was sure I posted a reply last night but then couldnt find it... 2 patches :p

Can you actually split them back to separate patches (and let me know which depends on the other?) its better we do 1 fix per commit incase anything goes pear shaped.

If the menu thing is such a problem we can probably work something out, I dont think its such a problem though./

and no, I havnt had a play with serial and the mini... I've got to much already on my hardware todo list :p
Comment by Laurent Gautier (Creposucre) - Friday, 13 November 2009, 13:31 GMT
Ok, I did another patch for the fm-remote.

I think it's better to commit this one first, I'll update the dock patch as soon as this one is in svn.
Comment by Laurent Gautier (Creposucre) - Wednesday, 02 December 2009, 12:33 GMT
Patch commited in R23805
Comment by Rosso Maltese (asettico) - Saturday, 09 January 2010, 15:41 GMT
@Creposucre (http://www.rockbox.org/tracker/task/10494#comment33165)
Also remember to activate the accessory supply in the system settings menu. :-)

Also a question: since some time the FM is supported by the trunk FW, what are the differences the this patch implement?
TIA
Comment by Laurent Gautier (Creposucre) - Tuesday, 12 January 2010, 22:39 GMT
This patch implement nothing more than what is in the trunk.

We should close this task, I'll open another one for further Improvements.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 12 January 2010, 23:04 GMT
Laurent, next time you are online ask Zagor or B4gder to get you developer rights on the track so you can close out bugs.

Loading...