Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category FM Tuner
  • Assigned To No-one
  • Operating System iAudio X5
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by RaeNye - 2006-05-11

FS#5347 - FM radio for X5: almost there

It appears that the Philips FM tuner driver works for the D&A 202 too;
Only missing is the part that calls the audio multiplexer and enables “recording” on the TLV320.

(So how do I know it’s working at all? Since I can scan for FM presets and it locks on actual stations!)

Note that the patch might not compile cleanly against current cvs since I changed many files and only uploaded those relevant to this patch. YMMV.

Closed by  linusnielsen
2006-07-21 08:43
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

I fixed the context menu bug and committed the patch. Thanks for your patience.

OK, now I can hear the radio as well.
Volume doesn’t change, though.

Tobu commented on 2006-05-15 01:17

Apparently the patch doesn’t use the unified format all the way; remember to pass -u so that it can apply.

Resynced to current CVS, also cleaned up a little.

Note that this patch now includes the recording patch v2 (FS #5355) as a part of it (both patches touch the very same things, so it would be a PITA to apply one on top of the other anyway). Thanks Andy.

Known bugs:
1. If you exit the FM radio by visiting the main menu and exiting it rather than by pressing the POWER button, the FM mux is still on so you won’t hear the digital audio. To fix this, exit FM mode properly.
2. The context menu (long SELECT) seems to automatically select the first menu item when enterring it. It is a result of making SELECT button function as an alternate MENU_ENTER button, and I don’t know how to fix this. To circumvent it, do a long SELECT press and without releasing the joystick, press DOWN (or UP). Ugly but usable.

Project Manager

Good work!

It was a little hard to review the patch, since you have made lots of irrelevant changes to the code, especially whitespace changes in i2c.c and pcm_record.c. Could you please update your patch to include only relevant changes?

Actually i2c-coldfire.c has changed quite a bit, so it’s better to view it as a whole. The whitespace changes are minor.
The changes in pcm_record.c are from the recording patch, so I just synced them.

However, I manually edited the .patch to remove the ~10 affected lines. I didn’t check it still applies correctly, as I’m not on my env.

Project Manager

The whole idea of CVS is to track the changes, so it’s still important to only change the relevant stuff. I’m not really sure why you feel compelled to change the indentation in i2c-coldfire.c for no reason. If you had left the indentation as it was, it would have been much easier to see the changes.

Besides, the CONTRIBUTING file says that you should keep the formatting when you edit a file, which means that you should not change the indent level.

But I changed the entire i2c-coldfire implementation!
I’m not compelled to maintain indentation level when /rewriting whole functions/, do I?

I also prefer two space indents, but the CONTRIBUTING file compels us all to indent with four spaces:

“Always indent your code with four spaces. Don’t use TAB characters, as that will mess up code display in CVS, printing, and a zillion other places.”

Ready for commit:
- resynced to CVS - tabs eliminated (I still have to figure out why vi puts tabs in it)
- tested compilation on x5, h1xx, h3xx, ipod photo and archos player

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing