Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.7.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by BenjaminBrown - 2011-01-22
Last edited by bluebrother - 2011-02-27

FS#11899 - logo.c keymap defaults instead of error on undefined targets

Starting at line 218 logo.c defines keymaps to any target that did not explicitly get defined beforehand. Here is patch that adds defines for the the missing targets and errors out when it gets compiled for an unsupported target just like all the other plugins.

Closed by  bluebrother
2011-02-27 16:42
Reason for closing:  Not a Bug
Additional comments about closing:  

I read the last comment as request for closure, so closing it. If I'm mistaken on this please request a reopen.

I realize that changing such generic catch all code can be fatal I suspect it was left over from some time ago when there were just a few targets. I am half way done testing on all the sdl sim builds, where I can, and have not had any trouble compiling.

Here are my test results for this patch. If this is acceptable to any devs, I would like to see this patch committed So I can move ahead dissecting FS#11898 into smaller manageable patches and finally get the plugins compiling nicely for android.

Your file mentions that logo.rock doesn’t run on the iHP-100/110/115 series. Does it work without your patch?

the results show only the simulator build failed but every single other plugin error catches targets without keymaps. Logo wants to assign them arbitrary ones which can cause problems. I will check again just for you.

I retested and my mistake it seems to build the simulator quiet nicely for theiHP-100/110/115 and logo is running perfectly well.

Looking over everything this change is not mandatory for FS#11898 at all and can be considered solely for the benefit of better code. So sorry for the confusion and extra time I spent on it, ha I’m a dork.

Will commit it anyway :)

My h100 does have logo.rock and it runs (r29079) without any modifications of logo.c. Is this patch only relevant for simulator builds?

It’s relevant to all builds the old code caught every target without a defined keymap, But really considering this patch further I concede that leaving as is will keep it a safe bet come compile time. Close this patch its unneeded.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing