Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category FM Tuner
  • Assigned To No-one
  • Operating System iPod 5G
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Laurent Gautier - 2009-08-07
Last edited by Jonathan Gordon - 2010-01-12

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

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

Closed by  Jonathan Gordon
2010-01-12 23:04
Reason for closing:  Fixed
Laurent Gautier commented on 2009-08-07 19:23

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…

Thomas Martitz commented on 2009-08-07 22:29

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.

Nils Wallménius commented on 2009-08-08 11:06

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.

Rosso Maltese commented on 2009-08-08 11:22

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’

Laurent Gautier commented on 2009-08-09 10:19

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

Rosso Maltese commented on 2009-08-09 11:38

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)

Laurent Gautier commented on 2009-08-09 19:28

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?

Jonathan Gordon commented on 2009-08-10 06:15

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

Laurent Gautier commented on 2009-08-10 07:55

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?

Jonathan Gordon commented on 2009-08-10 16:10

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?

Laurent Gautier commented on 2009-08-10 17:07

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

Jonathan Gordon commented on 2009-08-10 17:17

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

Laurent Gautier commented on 2009-08-10 17:32

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

Jonathan Gordon commented on 2009-08-10 17:41

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

Laurent Gautier commented on 2009-08-10 18:49

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 :)

Jonathan Gordon commented on 2009-08-10 19:17

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)

Laurent Gautier commented on 2009-08-13 14:37

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…

Gman commented on 2009-08-15 05:40

Hi I just got my iPod FM Radio today so if I can get SVN to work I can test this out.

Laurent Gautier commented on 2009-08-15 08:17

You bought it especially for that? Man, you put pressure on me :-)

Antoine Cellerier commented on 2009-08-28 18:08

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)

Laurent Gautier commented on 2009-08-29 09:00

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)

Laurent Gautier commented on 2009-08-31 16:39

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?

Antoine Cellerier commented on 2009-09-02 21:52

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?

Laurent Gautier commented on 2009-09-03 07:58

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.

Rosso Maltese commented on 2009-09-04 13:46

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.

Rosso Maltese commented on 2009-09-04 13:53

Sorry, forget the previous attach.
svn diff DO NOT consider new files!!!

Laurent Gautier commented on 2009-09-04 16:55

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…

Rosso Maltese commented on 2009-09-05 21:27

Just another fix about calling tuner_get_rds_info() in apps/recorder/radio.c when building for simulator.

Antoine Cellerier commented on 2009-09-09 21:52

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?

Laurent Gautier commented on 2009-09-10 12:05

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?

Laurent Gautier commented on 2009-09-28 16:45

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

Rosso Maltese commented on 2009-10-17 11:07

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.

Laurent Gautier commented on 2009-10-17 21:42

Synced with 23234.

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

This patch includes the RDS support.

Please report your feedback

Rosso Maltese commented on 2009-10-24 17:54

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?

Laurent Gautier commented on 2009-10-24 21:25

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.

Laurent Gautier commented on 2009-11-11 18:52

Synced with 23610

Jonathan Gordon commented on 2009-11-12 06:30

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.

Laurent Gautier commented on 2009-11-12 21:22

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?

Jonathan Gordon commented on 2009-11-12 21:40

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

Laurent Gautier commented on 2009-11-13 13:31

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.

Laurent Gautier commented on 2009-12-02 12:33

Patch commited in R23805

Rosso Maltese commented on 2010-01-09 15:41

@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

Laurent Gautier commented on 2010-01-12 22:39

This patch implement nothing more than what is in the trunk.

We should close this task, I’ll open another one for further Improvements.

Jonathan Gordon commented on 2010-01-12 23:04

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...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing