This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#10287 - rbutil - create a class for target specific static info
Attached to Project:
Rockbox
Opened by Dominik Wenger (Domonoky) - Saturday, 06 June 2009, 17:59 GMT+2
Last edited by Dominik Wenger (Domonoky) - Wednesday, 31 March 2010, 22:41 GMT+2
Opened by Dominik Wenger (Domonoky) - Saturday, 06 June 2009, 17:59 GMT+2
Last edited by Dominik Wenger (Domonoky) - Wednesday, 31 March 2010, 22:41 GMT+2
|
DetailsThis patch aims to create a class/target tree to store target specific static information. So all target specific information can be stored at one place and be translatable.
In the end this should replace the target specific info from rbutil.ini. Please comment.. 
rbutil-target.patch
(36.4 KiB)
rbutil/rbutilqt/base/bootloaderinstallbase.cpp | 46 --------------- rbutil/rbutilqt/base/bootloaderinstallbase.h | 9 --- rbutil/rbutilqt/base/bootloaderinstallhex.cpp | 14 ---- rbutil/rbutilqt/base/bootloaderinstallhex.h | 1 rbutil/rbutilqt/base/target/target-gigabeatf.cpp | 45 +++++++++++++++ rbutil/rbutilqt/base/target/target-gigabeatf.h | 39 +++++++++++++ rbutil/rbutilqt/base/target/target-h100.cpp | 57 +++++++++++++++++++ rbutil/rbutilqt/base/target/target-h100.h | 39 +++++++++++++ rbutil/rbutilqt/base/target/target-h120.cpp | 57 +++++++++++++++++++ rbutil/rbutilqt/base/target/target-h120.h | 39 +++++++++++++ rbutil/rbutilqt/base/target/target-h300.cpp | 57 +++++++++++++++++++ rbutil/rbutilqt/base/target/target-h300.h | 39 +++++++++++++ rbutil/rbutilqt/base/target/target-iaudiom5.cpp | 43 ++++++++++++++ rbutil/rbutilqt/base/target/target-iaudiom5.h | 39 +++++++++++++ rbutil/rbutilqt/base/target/target-iaudiox5.cpp | 43 ++++++++++++++ rbutil/rbutilqt/base/target/target-iaudiox5.h | 39 +++++++++++++ rbutil/rbutilqt/base/target/target.cpp | 69 +++++++++++++++++++++++ rbutil/rbutilqt/base/target/target.h | 66 ++++++++++++++++++++++ rbutil/rbutilqt/rbutilqt.cpp | 9 +-- rbutil/rbutilqt/rbutilqt.pro | 24 ++++++-- 20 files changed, 696 insertions(+), 78 deletions(-) |
This task depends upon
Closed by Dominik Wenger (Domonoky)
Wednesday, 31 March 2010, 22:41 GMT+2
Reason for closing: Out of Date
Wednesday, 31 March 2010, 22:41 GMT+2
Reason for closing: Out of Date
- is it really necessary to duplicate getValue(enum ETargetValue key) in all subclasses?
- The classes for h100 and h120 are almost (completely?) identical. While integrating rbutil.ini will require two distinct classes I'm wondering if both could share the common code and strings. Having duplicated strings will also duplicate them when translating. Maybe it would be a good approach to also distinguish between manufacturers (as in base class -> iriver -> h100) to have a place for common manufacturer strings?
- minor nitpick: the patch adds at least at one line trailing spaces. I'd prefer not to do so ;-)
- Idea of manufacturer classes in between is good. I will try this.
- Now there is only one getValue function. And the combining of those lists happens via constructor chaining.
- Its now easy to create classes above the actual targets to remove dublication. But i a unsure which schema to choose. Manufacturer doesnt really fit, for example h10 and h120 are pretty different. Cpu model also isnt right, as we have pp targets whith pretty different install methods.
So i choose to combine manufacturer and cpu/soc model for those targets where we have different ones, for other targets we can just use manufacturer or even no other parent class if not needed.
Example:
Target -> TargetIriverColdfire -> TargetH120
Target -> TragetIriverPP -> TargetH10
Target -> TargetIaudio -> TargetIaudioX5
Target -> TargetGigabeatF
- removed trailing spaces.
Even more rework :-)
- Moved manufacturer specific files into sub directorys.
- Added another class to irivers. Its now:
Target->TargetIriver-> TragetIriverColdfire -> TargetH120
- Renamed the Iaudo base class to TargetCowon
- Add Name and Brand from rbutil.ini to show how it would fit in.
- Added Archos player target.
So i moved all into one file. This should improve the concentration of the info, so nothing gets forgotten, when you add a new Target.
I am also thinking about adding more things to this Target class, ie info about the currently selected device (is rockbox installed, which version, voicefile etc, which would need to be scanned after selecting a new Target.) Also the list of all targets known to rbutil should go there. (Including targets where rockbox or rbutil doesnt work, so we can give better info).
Please comment.