Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category FM Tuner
  • Assigned To No-one
  • Operating System Gigabeat F/X
  • 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 nls - 2008-12-07
Last edited by nls - 2008-12-09

FS#9609 - Gigabeat S FM radio support

Fm radio support for gigabeat S.
It works but seeking isn’t nice yet and should really use the si4700’s built in seek functionality.
Muting doesn’t work for some reason and the keymap can probably be improved.

edit: forgot to say that i don’t know what audio_set_output_source() should do and im’ not quite sure about what i’m doing in audio_input_mux() is correct.

Closed by  nls
2008-12-09 20:49
Reason for closing:  Accepted
Additional comments about closing:  

committed

MikeS commented on 2008-12-07 22:10

audio_set_output_source sets the input that that output is connected to (ie. which input is heard) so it is possible for instance to process audio from a source and play it back through software (audio_input_source might be FM while audio_output_source would be playback) would rather than directly piping something direcly back to the output.

+ unsigned int spacing = spacings[write_bytes[7] & (3 « 4)] ;
should be [(write_bytes[7] » 4) & 3]

The datasheet mentions that some bytes should be not modified when written (first read and then written back).
Doesn’t it make more sense to read all the registers (in a si4700_init() for example) and then modify individual bits, instead of using a built in list of registers values?

nls commented on 2008-12-08 12:50

New version, implements detection, I don’t know if there were any gigabeat S’s that didn’t have radio though.
Fix the array indexing bug funman spotted and implement the suggested si4700_init().

Edit: updated patch without stupid mistake :)

spotted other things :

* you should read all the registers to a temporary buffer and copy to write_bytes, because you start reading from register 0xA and write to 0×2 * you should write your copyright in si4700.c ;)
* the #endif comment of the si4700 case in tuner.c is for another tuner
* I don’t see the power up bit set in si4700_init() (I see it in si4700_set() however)
* si4700_init() is indented on 5 spaces

Also are the GPIO interrupts needed?

nls commented on 2008-12-08 19:28

New version fixes funman’s comments except powering up in the init,
it should remain in the RADIO_SLEEP case in si4700_set.
Make the detection actually work, leave softmute enabled, lower the output
volume to be a little closer to playback, fix a couple of mistakes.

nls commented on 2008-12-08 21:44

Fix an off by one error, should use 10 bits for setting channel, thanks to bertrik for spotting!

Works well on my Gigabeast, save for two quirks:

1) The FM radio’s volume is quite loud, much louder than normal playback should be.

2) Entering the main menu from any other screen seems to take a second or two longer than it should.

Other than that, great job! :)

New version of the patch, with a few fixes by myself and Nils after some brainstorming on IRC:

1) Fixed a small bug in si4700.c, where returning to the main menu from any other menu or the file viewer would take a second or two longer than it should; this was done by moving fmradio_i2c_read from si4700_get to the RADIO_STEREO case.

2) In audio-gigabeat-s.c, fixed the passthrough volume to be a lower value; originally it was set to 7, which was causing the radio to be much louder than normal playback (passthrough volume was +6dB at this point). Setting it to 5 now sets the passthrough volume to 0dB, which is much more sensible.

3) I set the VolUp and VolDown buttons to work as they do in the WPS on the FM radio screen as well. Please help test this.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing