• Status New
  • Percent Complete
  • Task Type Patches
  • Category User Interface → Themes
  • Assigned To No-one
  • Operating System Another
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by mcuelenaere - 2010-09-22

FS#11639 - Android port: add theme downloader activity

This patch introduces 2 new activities, one which lists available themes for a specified target and another which lists more details and the option to install the specific theme. Currently this is more a proof-of-concept, as the communication with the theme server is handled by the same API RBUtil uses, which isn't perfect for the intended usage (e.g. offers no possibility to filter by screen size, only uses INI as output format, ..).

(this also includes some minor unrelated changes to the main Rockbox extraction functionality which won't be in the final patch)

- make translatable
- better UI
- uninstall/update functionality
- show only themes fit for the displays' resolution
- use different protocol to communicate with theme server to get rid of iniparser dependency (XML, JSON, .. ?)

The themesite API already does filter for the correct screensizes in the same way as it does for rbutil.
You just need to give it the correct target name and it will list only themes for this target. Ofcourse the themesite then also needs to have the correct android targets set up.

And what is the problem with the INI format ? Its very easy to parse and to create on the themesite. No need for complex xmls.

I think it'd be much easier to just pass the LCD resolution to the theme site, that way we don't need to create a lot of Android targets with different resolutions.

The INI format isn't a problem per se, it just requires an extra dependency in Java (AFAIK) compared to e.g. XML or JSON, for which there is support in the framework.
Also, I'm not sure about the license of the IniParser class I'm currently using, but that's not really relevant.

Sometime ago, rbutil did send the screen resolution to the themesite, but this was changed. Probably because screen size isnt the only thing which defines if a theme works on a target or not.
Currently the themesite decides based on the results of checkwps if a theme works on a specific target.

With the ini format its the other way round for rbutil. XML would need a extra dependency and the Ini format parser is built-into Qt.
And google just told me, JAVA can handle ini file nativly as it is the same format as .properties files. Take a look at this Webpage:

But we could ofcourse create another API on the themesite only for the android targets. Feel free to hack the themesite, its in svn :-)

Yes, I didn't mean modifying Rockbox Utility but rather fork the current rbutilqt.php page into an Android-specific one :)
And I didn't know about that API, I'll take a look at it.

I think only sending the screen resolution to the theme site, which then looks up all touchscreen-compatible themes with that resolution, should be doable.

I'm not sure if such a complete solution is needed. Couldn't we just auto-download cabbiev2 on the first start, and the other themes would be installable by rbutil as with the other targets?

I wouldn't say this is *necessary*, but it could be a nice feature.

Also, smartphones are getting more and more close to computers feature-wise; they're perfectly capable of downloading a file from the internet and unzipping it.

I, for example, wouldn't want to use RButil for just installing themes, something which could be done perfectly on the phone itself (hence the patch :) ).

Right, but only because we're not on daps that doesn't mean we can start bloating Rockbox with unnecessary (as you said yourself) stuff. I'm not a huge fan of a complete theme browser or some other replacement for rbutil.
Perhaps it could work better as a separate app?

1) I don't consider this as bloat, but I guess this these are just 2 personal opinions
2) this isn't "bloating" main Rockbox, as it only touches the Java parts (contrary to my dynamic screen size patch) thus unaffecting other targets; this would merely mean an increase in apk size (and possibly RAM size)
3) making this a separate app would require having the .rockbox directory writeable to other apps, and from a security POV only apps allowed by Rockbox should access these. After a quick google, I couldn't find a way to selectively allow acces (only an all-or-nothing solution exists, by chmod'ing the files world read-/writeable).

I guess it's up to the (developer) community to decide whether we'd want this in or not. In any way, I'll try to get this cleaned up and committable this week.

I havnt tested this at all, and dont really see any problem with putting this sort of thing in the apk, however regarding point 3, user themes should be downloaded to /sdcard/rockbox(?) so there should be no problem with shared write access. Does this show up as a seperate app in the app list or do you have to get it from inside rockbox?

AFAIK we agreed on storing .rockbox etc. in the private data folder, or will Rockbox fetch some files from the data dir and some from the SD card?

Currently it's an item in the options menu in the main Rockbox activity, which launches the ThemeListActivity activity.

I'm not aware of any such agreement. And it's not how it currently works (everything that's in the zip is installed in the private folder, but all files rockbox creates/writes are on the SD card. And all optional files (like a custom viewers.config) are also on the SD card.

We can surely discuss this and possibly change things, I'm just noting that I'm not aware of any agreement.

This is an updated version of the old patch, changes are:
* Got rid of IniParser dependency and replaced the themes backend on the site with a new one (using JSON)
* Moved the app to a separate project (whether this needs to be in Rockbox SVN is debatable, but I'd like it)

* Implement theme scanning and Theme Management Activity
* Improve/Fix image loading in DrawableManager/ThemeListActivity
* Implement JsonReader which adds parsed themes from the raw JSON text on-the-fly to the list (optional)

I think it would be a better idea to have a separate downloader app. This can be especially useful on devices with small flash memory where you can simply remove the theme downloader functionality after using it by uninstalling the apk. Rockbox could detect such a downloader app and provide some kind of way to invoke it directly.

I'm not sure if you have looked at my latest patches, but this is a separate Android app in utils/android_themesmanager/.
Whether Rockbox should detect the app and offer a way to launch this, I personally don't see a need for it and thus wouldn't use it. Feel free to implement it though :)


Available keyboard shortcuts


Task Details

Task Editing