This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#12402 - add mkimxboot as a new method in Rockbox Utility
Attached to Project:
Rockbox
Opened by Jean-Louis Biasini (JeanLouisBiasini) - Wednesday, 23 November 2011, 18:04 GMT+2
Last edited by Dominik Riebeling (bluebrother) - Wednesday, 04 January 2012, 20:19 GMT+2
Opened by Jean-Louis Biasini (JeanLouisBiasini) - Wednesday, 23 November 2011, 18:04 GMT+2
Last edited by Dominik Riebeling (bluebrother) - Wednesday, 04 January 2012, 20:19 GMT+2
|
DetailsSo I will try to implement that... but I'm really beginning with C++
|
This task depends upon
Closed by Dominik Riebeling (bluebrother)
Wednesday, 04 January 2012, 20:19 GMT+2
Reason for closing: Accepted
Additional comments about closing: A reworked version of the latest patch has been committed.
Wednesday, 04 January 2012, 20:19 GMT+2
Reason for closing: Accepted
Additional comments about closing: A reworked version of the latest patch has been committed.
by now the problem is: https://gist.github.com/1389121
that can be turned around once by adding:
#ifndef MKIMXDUALBOOT_H
#define MKIMXDUALBOOT_H
#ifdef __cplusplus
extern "C" {
#endif
#ifdef __cplusplus
};
#endif
#endif
to the file rbutil/mkimxboot/bootloader.h
but:
1) rbutil/mkamsboot/bootloader.h file doesn't have any of this so I must be missing something...
2) it will compile only once and will end with: https://gist.github.com/1388994 on any other build attempt
the 2 new files are probably wrong but should compile
I've updated your patch to fix this. I'm not entirely happy with the fix ($LIBS gets a bit too long for my tastes), but everything compiles
Jean-Louis: I'll try to give this patch a closer look tonight. Please try to avoid adding trailing spaces :)
But yes, that is indeed the underlying problem. I think they all need to be updated to put their objects somewhere more specific. Just changing mkimxboot will fix this particular issue, but we should fix this once and for all.
For what I understood from the code: installation methods are supposed to follow a quite thin path: take the bootloader download path and OF local path that rbutil give then throught Bootloaderinstallbase class and then do whatever it takes to patch and install. As the bootloader isn't yet on the server (pamaury would like to wait a little to be sure it doesn't need any change) I was supposed to implement something asking the user for the 2 files (bootloader + OF). The methods doesn't seems to work like that. Should I implement something on a upper level, to have bootloaderbase's class able to handle it? Or should I just I just wait for the bootlloader to be on the server?
I've started working on restructuring the lib Makefiles to have a proper way of avoiding the filename clash.
rbutil/rbutilqt/build//libmkimxboot.a(mkimxboot.o):(.rodata+0x68): undefined reference to `dualboot_fuzeplus'
collect2: ld returned 1 exit status
make: *** [RockboxUtility] Erreur 1
collect2: ld returned 1 exit status
make: *** [RockboxUtility] Erreur 1
I noticed a warning during compilation:
base/system.cpp: In static member function ‘static QMap<unsigned int, QString> System::listUsbDevices()’:
base/system.cpp:249:9: warning: variable ‘res’ set but not used [-Wunused-but-set-variable]
base/system.cpp:250:13: warning: variable ‘count’ set but not used [-Wunused-but-set-variable]
- I tested the file upgrading my device with and it works.
- anyway I wasn't able to have the same md5sum as with the standalone mkimxboot. The std output while running rbutil from console give also strange stuf at the end. I suppose the this is related to the debug fonction but I'm not quite sure about it... It would be wise that someone check that...
- add more debug info and display more info while installing
- remove an unused include
- correct url in the hint that was wrong
- I noticed that I often get random error on patching or even moving the files! Sometime it works flawlessly, sometime not. Very strange... But this seems to works everytime on the first attemps. I suspect some temporary files are messing up
- the problem with random error is solved: it was related to the name of the temporary file writed to disk before being copied to the device. No more errors now.
- Another bug was related to the fact that and existing bootloader file on the device was rename to .orig. But if such a file also already exist it was leading to a copy faillure... Anyway this security check is not needed because such a file on the device is not the bootloader itself but just a file to update the bootloader. We can safely remove it unstead of rename it: in the murphy user senario the user will just need to download a new OF.
- implemented hint: reminder to have enough battery (before installing because once the firmware is on the device any deconnection would start updating)
- implemented posthint: splited a general hint to have a specific hint on how to boot in OF mode from an already rockboxed device. this might seems a little messed up but this is the only way to reuse some part of the already written hints while having all hints showing up in the right order for the fuze+
1. For getting the basename you can use QFileInfo::baseName() instead of splitting a QString. Splitting a QString might fail if it's using other path separators than the one specified (could be an issue on Windows).
2. You retrieve a char* to the filename. However, I'm not sure how long such a pointer is still valid. You should pass that to the mkimxboot() call directly, as done with m_offile. Using a hadcoded filename as done in v7 is in general a bad idea.
3. around the middle of installStage2() you use a rather weird conversion of m_blfile from QString via a 8bit encoding to a QString again. m_blfile is already a QString.
4. In installed() we can look for the .orig file and use that to determine if a Rockbox bootloader is installed. Also, is there any magic in the patched firmware file we can check the firmware file for our bootloader?
5. There is no need to flush() the patched bootloader file before close()ing it.
6. I don't think we should put instructions on how to dualboot into the bootloader installation. We don't do this for other players. Alternatively we could add it for all players (though I'd prefer to keep that information in the manual -- a pointer to the manual might be a good idea though)
7. We should avoid abbreviations like "OF" in user messages -- that's Rockbox lingo and most users are unlikely to be familiar to that. For the same reason I'd like to avoid using "rbutil" in our support channels :)
2. Ok I wasn't sure what was the cleaner and more readable way to do it. I've taken away all those char* then
3. right another useless convertion
4. The file copied to the device get erased after update. So we cannot rely on it for detecting rbbootloader/version. I also ask Pamaury if it would be possible to have some header in RB booloader with some version. He didn't seems very enthousiastic, and it will be a lot of work to versionning all the different patched bootloader version... A solution could be to retrieve the date the bootloader's file was added to the server. We could write it into some versioning file on install.
Then before patching we could check this file (no file == unknow or new install) and compare it to the file from rb server to see if a more recent was not already installed...
5. ok
6. I thing that it should be added for all device: people are not going to look at the manual 2 seconds before being able to try RB. This basicly means that they are going to ignore the link and show up directly on the forum and irc saying: "your stuff doesn't work!" :D The problem is that there is nothing general here, booting into OF can be the same way on a group of target, specific to one player or a combination of both...
Idealy I suppose we should have a class in bootloaderinstallbase to send message from the different install method. We could have standardized message and set the one we want in some argument + the possibility to send specific message... Having player specific code in base is not really in the spirit, is it?
For now the pointer to manual seems the simplier solution anyway... except we don't have manual yet for Fuze+! :D
Actually this is the very same problem with the recommandation I added to ofHint about battery to be over 70%. This is quite important to remind it to the user but this has nothing to do with off
7. right
Thank you very much for your comment and help on this! :)
6. When pointing to the manual for all devices we need to handle the case that some players don't have dualboot (the iaudio ones AFAIK). That means that either the manual needs to have a specific section about it that is also present for those (like the current "Starting the original firmware" section) or we need to distinguish in Rockbox Utility. Putting it explicitly in the manual is better IMO because it makes it clear that there is no way to boot the original firmware on those players.
5. yes the fact to have some "Starting the original firmware" on every target seems the better solution. this means I'm going to go on with fuze+ manual first (I would mike to implement 4. first)
5. I agree. I'm not sure what's the best way to link that, since there is no guarantee that deep links to the manual won't change -- the simplest way should be to simply indicate the user to read the manual if he wants to start the original firmware.
5. That's right. It is simpler. We'll see further if it's really needed then.
I'll post the last modification (just the rtm stuff) soon and then go on for the manual.
this should be ready for the svn now... (at least as soon as we'll get a bootloader on line)