Rockbox

Tasklist

FS#10287 - rbutil - create a class for target specific static info

Attached to Project: Rockbox
Opened by Dominik Wenger (Domonoky) - Saturday, 06 June 2009, 15:59 GMT
Last edited by Dominik Wenger (Domonoky) - Wednesday, 31 March 2010, 20:41 GMT
Task Type Patches
Category Rbutil
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This 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..
This task depends upon

Closed by  Dominik Wenger (Domonoky)
Wednesday, 31 March 2010, 20:41 GMT
Reason for closing:  Out of Date
Comment by Dominik Riebeling (bluebrother) - Saturday, 06 June 2009, 23:06 GMT
Just had a quick look:
- 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 ;-)
Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 11:44 GMT
- The problem with the getValue() function is, that it has to use the valueList of the child class. So if we dont override the function in every class, it will use the wrong list. But i will see if i can find a better solution.
- Idea of manufacturer classes in between is good. I will try this.

Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 13:00 GMT
I found a solution for problem one.
- 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.
Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 13:35 GMT
- inverted order when searching for values. So Child classes can override values.
Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 14:15 GMT

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.
Comment by Dominik Wenger (Domonoky) - Thursday, 25 June 2009, 13:49 GMT
I somehow didnt like those many many files.
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.

Loading...