Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Rbutil
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.2
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Domonoky - 2009-05-03
Last edited by Domonoky - 2009-08-15

FS#10185 - rbutil : bootloader support for sansa AMS targets

This patch is the beginning of bootloader support for AMS targets in rbutil.

The modifications needed, will go into different patches.
- General Bootloader class rework to better support the ams targets.
- Modifications to mkamsboot, so its better to use in rbutil
- BootloaderInstallAMS class.
- rbutil.ini modifications for the new targets.

First patch: general Bootloader class rework:
- renamed NeedsFlashing into NeedsOF and moved the Hint for this into the Bootloader classes, also setfunction for OF file is now in the base class.
- Enforce proper parents for all bootloaderinstall classes
- Make some functions, which must be implemented, abstract.
- move the postInstallHints to the child classes (and non-static)

Next patches will follow.

Closed by  Domonoky
2009-08-15 13:05
Reason for closing:  Accepted

I’m wondering why we can’t have rbutil downloading the OF. They’re distributed on Bagder’s site, aren’t they?

We cant distribut the original Firmwares, as we dont have a licence to distribute them.
What Badger does on his own Site is his own problem. :-)

And we also request the User for a OF for the Hxx0 installs, so that shouldnt be a problem.

We can still make rbutil download it, not?

- I’m not sure if renaming NeedsFlashing is really needed – we put a file on the player that gets flashed, so that term isn’t wrong. OTOH, the X5 / M5 / M3 also flashes a file and it doesn’t require an OF file, so installation handled by the rbutil file class, so NeedsOf might more exact.
- I disagree with moving the postInstallHints to the installation classes. The idea behind making it a static function was that the installation classes should *not* know (and not need to know) about the player model they are installing. Ideally the postInstallHints would get moved out of the bootloader classes completely, but having it in the base class as static function is a compromise for better reuse once we get a command line interface. We could, however, consider a general “hints” class that only returns such strings.

- I think the renaming is needed, to make clear what it really does. This flag does nothing Flashing specific, but states that it needs a OF File.

  1. this together with the ofHint in the class, makes it easier to add similar classes.

- about the postInstallHint, i agree that it should be made static again. (maybe a comment could have made the intend of this clearer :-) ).

New patch in this series: Modifications to mkamsboot.
- The code was reorganised so the main consitsts now of only a few steps.

  1. load, check and pack the of,
  2. load, check and pack the bootloader
  3. patch it
  4. write out.

- There may be a few more functions needed to really do the same in rbutil

Work left for this:
- it needs to be built to a lib, excluding the main()
- The builtin dualboot code should be in svn as binary (so you dont need the arm-gcc to build rbutil)


								    
  			

Updates:
- I commited the general bootloader class changes with the changes bluebrother suggested.

New patch for mkamsboot:
- It can now be built as lib, but there is still the problem of the needed arm binarys. (currently i commented the arm rules out, so you need prebuild dualboot.o files)

Maybe some Makefile guru can help us out to improve this ? 

- Also more Makefile magic is needed for mac os X (ar has problems with multiplatform .o files (see rbspeex makefile) ). This also applies to tools/ucl/src/libucl.a.

New patch: InstallBootloaderAms Patch:
- It correctly makes a patched bootloader. But as there is no Bootloader on the download server it asks the user ( addition GUI depency, needs to be removed, when binarys are available).

New patch: Sample modifications for rbutil.ini to add e200v2 target for testing.

Comments on the rbutil patch:
- why the ofHint() in the bootloader class? That should go into rbutilqt.cpp as done for the “hex” install type.
- the bootloader install class must NOT utilitze any GUI elements (you #include <QtGui> in base/bootloaderinstallams.h (!) and use a file chooser). The file provided by the user has to get handled by the caller, i.e. in rbutilqt.cpp
- it would be good if you’d try to limit lines to around 80 characters. While rbutil deviates from the hard 80 characters limit of main Rockbox it would be good to at least try to follow it if possible. I’m fine with ignoring it in places where it doesn’t make sense (but in such cases it shouldn’d get exceeded too much).

I have merged your work on mkamsboot with other diffs in  FS#10253 

New patch, adapted to mkamsboot in svn.
Also included are the modifications to rbutil.ini to add the e200v2 target. Other AMS Targets only need similar entrys.

The code still includes the hack, to work around the non-existing bootloader on the server. But aside from that, it should be commitable.

mkamsboot and libucl probably still need some Makefile work to build as universal archive on Mac.

Thanks!

+ emit logItem(”No room to insert bootloader”,LOGERROR);
Perhaps should be
+ emit logItem(”No room to insert bootloader, try another firmware version”,LOGERROR);

and
+ emit logItem(”Uninstallation not possible, only installation info removed”, LOGINFO);
+ emit logItem(”To uninstall, perform a firmware upgrade with an unmodified original firmware”, LOGINFO);

Do you want a patch with these diffs (provided you agree) and .ini entries for Fuzev1 Clipv1 and m200v4 ?

Yes a new patch with this modifications would be nice.
And i agree, that those texts are better.

will do.

Also I wonder what happens if the user selects an original firmware not supported by mkamsboot (released after rbutil was built for example).

Will the error message only go to stderr or will it be displayed in GUI ?

EDIT: uh oh when I merged your patch I removed r21073 : “mkamsboot: really error out if OF model is different from bootloader model” , so untested firmwares will get accepted anyway.

It should send that error to the GUI

a few comments:
- the ‘extern “C”’ should be in the header, not the cpp file to avoid forgetting it when including from another file. #ifdef __cplusplus is your friend here.
- the patch contains unrelated changes (apps/plugins/SOURCES)

Hilton: “I wonder what happens” does NOT mean “how should it behave”. Not notifying the user about an error is is out of question, but how does this implementation behave if this situation occurs? I can’t see some general error-catching in the code but maybe I’m too tired right now.

Apply mentioned changes.

I didn’t add m200v4 USB ids.
Also I suppose “usberror” entry is for detecting if the device is in MTP mode ?

What about “usbincompat” ? e200v1 is “incompatible” with the Sansa Fuze..

Yes, the extern “C” could go into the mkamsboot header. But i dont see that as a big problem.
And about the error handling: did anyone here read the code ? It Outputs the same as mkamsboot in svn. Instead of stderr it uses the gui progresslogger.

About the usb-ids: Yes usberror is for a device in a wrong mode, and usbincompat for inkompatible Devices, for example a Ipod 6gen.

Thanks for the patch.

As suggested by Llorean on http://www.rockbox.org/mail/archive/rockbox-dev-archive-2009-06/0012.shtml , we could download a list of tested OF with their md5sums, rather than having the list built in mkamsboot.

That way rbutil could support OF tested after its compilation (new OF releases for example).

I am not sure which format we could use for this purpose: I suppose a simple text file would be easier to parse, something like
e200v2 3.01.11 1 e622ca8cb6df423f54b8b39628a1f0a3\n
fuze 1.01.11 1 cac8ffa03c599330ac02c4d41de66166\n

(model name, OF version, firmware file format, md5sum, all spaces separated and terminated by new line)

Yes, downloading the list of md5sums should be no problem.

About the content of the file:
we should probably provide the same info as mkamsboot already uses in the md5sum struct, then its easier to modify mkamsboot for it.
(mkamsboot standalone uses builtin list, and rbutil provides its own downloaded list)
So we should use the enum number instead of the modelname. (or maybe additionally)

About the format:
Your proposed format surely would work, and would be easy to parse. But even easier would be a ini format, because Qt has builtin support for ini (no parsing needed).
So something like:

[1]
modelname = e200v2
modelenum = 3
version = 3.0.1.11
fwversion = 1
md5sum = e622ca8cb6df423f54b8b39628a1f0a3
[2]
modelname = e200v2
modelenum = 3
version = 3.0.1.14
fwversion = 1
md5sum = 2c1d0383fc3584b2cc83ba8cc2243af6
[3]
….

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing