Re: 3.15 release
On Mon, Oct 28, 2019, 3:56 PM Franklin Wei via rockbox-dev < rockbox-dev@cool.haxx.se> wrote: > All, > > There has been some discussion between William and I on IRC regarding a > 3.15 release in the near future. We are currently aiming for a November 15 > release date. I will be serving as release manager. > > We plan to keep HWCODEC source and builds for this release (though some > builds may be dropped if they fail to build). A future 4.0 release will > mark the removal of HWCODEC. > > Thoughts? > > Franklin > Death of hwcodec? Yeah I don't believe it :) >
Re: Please help test gerrit#890
On 6 July 2014 05:28, Richard Quirk richard.qu...@gmail.com wrote: Please let me know if you find any crashes - or even better if this fixes any crashes with your troublesome skins! I didn't see any crashes, but the clock_lock2 theme I use on the WPS didn't work any more. This is on the sansa clip zip, here's a link to the theme: http://themes.rockbox.org/index.php?themeid=2144target=sansaclipzip Is there anything I can do to help out debugging it? I installed the patched rockbox, rebooted and it was back to the default theme. The rockbox fallback theme I think it's called. Applying the clock lock 2 theme from the settings made no difference, but it didn't crash. Cheers, Richard Thanks! This is fixed, please retry with the updated patch.
Re: Extending the metronome plugin
Hi On 19 June 2014 16:19, Thomas Orgis thomas-fo...@orgis.org wrote: 1. Is there an API call to get a file selection dialog or should the plugin instead register as handler/codec for tempo map files to be started from the main files menu? Excuse me for not finding that answer myself, I didn't find an obvious place where API/plugin design is documented. Pointers welcome, of course. I havnt done anything with plugins for ages but I'm pretty sure there isnt an API to choose a file. Your best option is to register as the extension handler (done in apps/plugins/viewers.config). Plugins have access to anything in the plugin_api struct (in plugin.h) and there are some extra stuff in apps/plugins/lib/ which you might find useful (or not!). 2. Is using the display as flashing indicator LED feasible? I guess it should be for the Clip+, as it's just a little grid of OLEDs. But the question would be if it can be done quickly in a timely manner to serve as metronome (thinking of switching the whole upper or lower half for opposing tick/tock). Since there is no background illumination, it should be rather power conservative to have all black pixels most of the time, right? And regarding the speed/latency: I remember running Doom on that display, so that should technically work;-) Sure, you can draw whatever you want to the display and control the backlight so go nuts! I'm a seasoned C developer (some might recognize me for maintaining mpg123 over the last 8 years) and am confident to get the job done for giving Rockbox an awesome metronome, once I got the starting points. I did read the source of the existing metronome plugin and would start from there. Alrighty then, Thomas
Re: Release
On 18 June 2014 16:25, Thomas Martitz ku...@rockbox.org wrote: I think we should still have a formal freeze period with RC builds. There are some open bugs that we can address too, perhaps. Alex is inactive so I don't think we have a release manager. Considering the state of the project (activity definitely seems low generally - just look at http://www.rockbox.org/since-4weeks.html) I think a freeze period is a waste of time. If someone is interested in freezing I would suggest just picking a commit and tagging it. Jonathan
Re: DevCon2014
So everyone is agreed on melbourne then? On 4 April 2014 04:35, Tomasz Moń deso...@gmail.com wrote: I'll definitely be there!
Re: Ladies and Gentlemen
On 2 December 2013 07:49, Thomas Martitz ku...@rockbox.org wrote: Hey folks, this is not the usual kind of Ladies and Gentlemen mail. Instead of a new port I have managed to get Rockbox play sound in a new environment. What I am working on is to to detach the playback core (including codecs, buffering and playlists) from the GUI and legacy OS-like (threads etc) environment. That means I want the playback core to work on native OS threads and synchronisation primitives (i.e. pthreads). I can say I have managed to get a few songs playing in such a environment. The first song of was I Could Be The One by Avicii / Nicky Romero. The code is a _huge_ hack right now (I really mean it) but I invite any of you guys to collaborate. I try to push some code as early as possible. Best regards. Sounds pretty cool! (Possibly a dumb question), Are you only targeting *as App type builds or will this be used on (i.e) ipods with the rockbox core?
Re: Soft lock and screen/lcd activation
On 12 March 2013 02:42, Amaury Pouly amaury.po...@gmail.com wrote: Hi all, As you know, many of our targets features a soft-lock in the WPS which locks all the keys except the unlock key, to prevent accidental changes. This is a very useful feature but recent targets like the Fuze+ have shown that it is too limited in two ways: 1) Only in WPS This feature only exists in the WPS screen, which kind of make sense if you listen to music but this is not the only screen where it would be useful. One of the most asked one is the radio screen where you currently cannot lock the keys. 2) Doesn't prevent screen activation Another problem which is more specific to the Fuze+ is that it features a touchpad which you can easily touch, and thus will constantly activate the screen/lcd. This is bad for the battery life and this is also very annoying. That's precisely why most phones on the market just won't respond to touch events when locked and you'll need to touch a physical buttons to activate it again. I think it would be very sensible to have such a feature in Rockbox too. To sum up, what many *users* would like to see is: * lock in radio screen (at least) * lock which disables the screen until a physical button is hit (with a setting) Maybe it is time to discuss about these issues and try to fix them. I would be willing to help but I must say this is mostly related to part of the code I don't know about so help would be greatly appreciated. Please share you opinion, Amaury Pouly There is nothing magical about making this work. Just OR ALLOW_SOFTLOCK to the fm screens action context and add a keymapping for ACTION_STD_KEYLOCK to the relevant keymap table. Now to make the backlight work how you want you just need to notify the firmware/ code that the buttons should be locked, add a softlock_buttons_lock(bool lock) to firmware/button.c and hook the backlight up to that. Jonathan
Re: RFC new line API
On 20 February 2013 22:34, Thomas Martitz ku...@rockbox.org wrote: I start with list.c because that's the most extensive user, but other screens can follow easily, e.g. various plugins. The API that I suggest can print the whole line (indent, spacing, icon and text) in a single call so it's generally attractive. Ignoring plugins, 99% of the UI is drawn by list.c, so yes adding API's for its use would seem attractive, but that will almost certainly end up being wasted effort. I'm really aiming to have the lists entirely drawn by the skin engine by the next release which would make any effort here redundant. So, in that spirit I'd advocate you just doing the semi-nasty hack and adding a bit to the style flags so the scroller can redraw the seperater line (or just piggy-back off the gradient num_lines bits). Once the skin engine is doing the list drawing the scroller can be returned to a dumb text drawer (all line decorations would be drawn onto the backdrop buffer so the scroller clearing the line would leave them intact.) If all you want is a line separator then why is it not a simple 1 line lcd_hline() call in the draw_list() loop? This is what the current version on gerrit does. The problem is that scrolling breaks it (the scroll engine redraws the line selector which clears the separator). And it sucks from a software-architecture POV: The line selector is currently drawn in firmware, the separator would be drawn in apps (in this scenario). So two completely different places although they are essentially very similar (style attached to each line). I really want to clean up this mess for good before moving forward with the separator. As above, in the interest of not wasting developer time I'd recommend doing the sucky way knowing it is temporary. Thinking about it now, it would be pretty cool to register a callback (per viewport) for the scroll engine to call which would do the line drawing instead of the current code. (It would have to be per viewport because it is very likley to have a skin scrolling line and the list scrolling line both scrolling at the same time, though a generic one would be needed for default which should just work). I have the beginnings of a scroll engine rework which does a callback (per line, and each line has a viewport associated). But even the current scroll engine already stores a viewport pointer for each line, that is set when the line is redrawn. This I'm interested in seeing. I'm starting to wonder if the scroller really needs the viewport pointer per line? If this was changed to store just the pixel rectangle to draw in (so the line number is also removed, and relative to the screen, not viewport) then it wouldn't necessarily overdraw the decorations. The callbacks becomes an issue, but I agree that using a callback is the way to go to remove this decoration drawing from firmware. Jonathan
Re: RFC new line API
On 18 February 2013 07:40, Thomas Martitz ku...@rockbox.org wrote: Am 16.02.2013 10:46, schrieb Jonathan Gordon: On 16 February 2013 08:30, Thomas Martitz ku...@rockbox.org mailto: ku...@rockbox.org wrote: Hello guys, I'm working on a new line print API in apps that's supposed to replaces most of lcd_puts_* and lcd_putsxy_*. The lcd_puts* became really messy and it still doesn't support scrolling properly (not at all for pixel based functions). The rework I'm working on will hopefully be simplified and fix scrolling, while allowing more control over the line style and contents. My motivation is to properly implement [1] (line separator in lists). This line separator cannot be properly implemented just in apps because scrolling draws over it, and scrolling is entirely in firmware and calls only firmware function. To not further complicate lcd_puts* a rework is needed. My idea is a having a single function: put_line(int x, int y, struct line_desc *desc, const char *fmt, ...). - x, y are the position of the line (in pixels!). - struct line_desc defines other properties the line: style (for line selectors, and the line separators mentioned above), scrolling, line height and multiline information. - fmt is a format string that controls the contents of the line. Similar to printf() tags can be used to put icons, text and margins (perhaps other stuff too in the future), the variable paramter list is then used to build the contents There are some complications, most notably scrolling, multiline and RTL, which I can hopefully work out soon (i think I will solve scrolling by setting up a callback so apps code can do the actual drawing). As if now there is a preview of my work in [2], see apps/gui/bitmap/list.c for an example of how to use the new api. So, what do you think? Is going forward worthwhile and welcome? Do you have comments/remarks/suggestions as to the proposed API function? Best regards. [1]: http://gerrit.rockbox.org/r/#/**c/384/http://gerrit.rockbox.org/r/#/c/384/ [2]: https://github.com/kugel-/**rockbox/tree/newline-apihttps://github.com/kugel-/rockbox/tree/newline-api Please don't add any non-text drawing to the lcd_put* API. That should do nothing but position text. I think you're going about this the wrong way (or at least doing too much at the firmware level). Sorry, I wasn't clear enough. My intent is to put put_line() into apps/. I totally agree that non-text stuff shouldn't be added to the firmware level, which is why I even make this work (to avoid putting the line separator to firmware which I *really* don't want). The current patch has it in apps/. Being in apps is also a requirement for extending the line selector to the icons and indent level (right now the line selector is only drawn below the text portion of a line), which is something I really want and already implemented. Eventually the line selector drawing (i.e. line styles) could be removed from firmware once a sufficient apps API is in place. I like the idea of registering a callback for scrolling lines, this would potentially fix existing issues with the list indenting, however I don't think this is the way to fix this (and your line separator). My suggestion is: 1) fix scrolling so it works on a pixel level and not a line level. We are well past the time when scrolling on a line level (and using the entire line) is outdated. If scrolling can be told the pixel rectangle (or sub viewport would be better/simpler) to scroll in then you are 90% of the way to what you want. This is on my TODO list, but 2) modify list.c to know the difference between the text height and line height. I'm guessing this is the actual issue you're aving with line seperater (though your line height patch hsold have done this anyway?) list.c already knows the difference, since the line height patch. What do you mean by this is the actual issue? list separator and line height are barely related. 3) Draw the line separator in the list UI which is the only correct place to put it. Sure, you could piggy-back off the gradient code and draw a line manually but please remember that everything in the UI that is drawn should be customizable (maybe not from the start, but certainly not modified to make it impossible later). (tangent: the gradient code is really in the wrong place too, the list should be drawing the gradient and then drawing the text over the top, not the lcd_* code). This is already implemented by the current version. So yes, the lcd_put* API is long outdated and crud, but changing it is orthogonal to your motivation. I don't want to change it. Not now at least. And I particularly dont want to expand it. This work makes some of lcd_put* obsolete so the API can be cleaned up in a follow-up change
Re: RFC new line API
On 16 February 2013 08:30, Thomas Martitz ku...@rockbox.org wrote: Hello guys, I'm working on a new line print API in apps that's supposed to replaces most of lcd_puts_* and lcd_putsxy_*. The lcd_puts* became really messy and it still doesn't support scrolling properly (not at all for pixel based functions). The rework I'm working on will hopefully be simplified and fix scrolling, while allowing more control over the line style and contents. My motivation is to properly implement [1] (line separator in lists). This line separator cannot be properly implemented just in apps because scrolling draws over it, and scrolling is entirely in firmware and calls only firmware function. To not further complicate lcd_puts* a rework is needed. My idea is a having a single function: put_line(int x, int y, struct line_desc *desc, const char *fmt, ...). - x, y are the position of the line (in pixels!). - struct line_desc defines other properties the line: style (for line selectors, and the line separators mentioned above), scrolling, line height and multiline information. - fmt is a format string that controls the contents of the line. Similar to printf() tags can be used to put icons, text and margins (perhaps other stuff too in the future), the variable paramter list is then used to build the contents There are some complications, most notably scrolling, multiline and RTL, which I can hopefully work out soon (i think I will solve scrolling by setting up a callback so apps code can do the actual drawing). As if now there is a preview of my work in [2], see apps/gui/bitmap/list.c for an example of how to use the new api. So, what do you think? Is going forward worthwhile and welcome? Do you have comments/remarks/suggestions as to the proposed API function? Best regards. [1]: http://gerrit.rockbox.org/r/#/**c/384/http://gerrit.rockbox.org/r/#/c/384/ [2]: https://github.com/kugel-/**rockbox/tree/newline-apihttps://github.com/kugel-/rockbox/tree/newline-api Please don't add any non-text drawing to the lcd_put* API. That should do nothing but position text. I think you're going about this the wrong way (or at least doing too much at the firmware level). I like the idea of registering a callback for scrolling lines, this would potentially fix existing issues with the list indenting, however I don't think this is the way to fix this (and your line seperator). My suggestion is: 1) fix scrolling so it works on a pixel level and not a line level. We are well past the time when scrolling on a line level (and using the entire line) is outdated. If scrolling can be told the pixel rectangle (or sub viewport would be better/simpler) to scroll in then you are 90% of the way to what you want. 2) modify list.c to know the difference between the text height and line height. I'm guessing this is the actual issue you're aving with line seperater (though your line height patch hsold have done this anyway?) 3) Draw the line separator in the list UI which is the only correct place to put it. Sure, you could piggy-back off the gradient code and draw a line manually but please remember that everything in the UI that is drawn should be customizable (maybe not from the start, but certainly not modified to make it impossible later). (tangent: the gradient code is really in the wrong place too, the list should be drawing the gradient and then drawing the text over the top, not the lcd_* code). So yes, the lcd_put* API is long outdated and crud, but changing it is orthogonal to your motivation. Jonathan
Radio Art is broken since may unfortunately)
Hi Mike, Can you please have a look at FS#12797 when you get a chance? It looks like da6cebb6b0b17b4a75a2bd4f51b7cf70b5dafe40 broke radio art (found by bisecting and testing in the fuzev2 sim). Thanks Jonathan
Re: Fix for default sleep timer duration as a shortcut
On 6 October 2012 05:47, Richard Quirk richard.qu...@gmail.com wrote: A couple of patches to do with the default sleep duration setting, the first adds a set_sleeptimer_duration function that will be used in the second. http://gerrit.rockbox.org/327 http://gerrit.rockbox.org/328 My use case is: I have restart sleep timer when switching on set to yes, and reset timer when touching any buttons set to yes. At night, I set the sleep timer to 15 minutes, and fall asleep at some point listening to music (maybe switching it back on if I don't fall asleep right away). Then in the morning I set the timer to a large value that it'll probably never have chance to reach during the day. That way I don't have to touch any other setting to activate/deactivate the sleep timer. 327 looks sensible enough.. 328 doesn't seem like the correct fix though, need to understand the issue more to comment.
Re: Archos Recorder build fails: too big
On 22 August 2012 00:50, Bertrik Sikken bert...@sikken.nl wrote: The archos recorder build has tipped over the limit for size, and now the autobuild always fails. The tipping point was commit bd6e6ed but the code has been growing steadily so I wouldn't say this particular commit is any more responsible than others before it. Still, we need to fix this somehow. Maybe someone could take a look at recent features and decide if some of them might be disabled for the recorder? Other suggestions? As I understood from some brief discussion about this on IRC, it can also be fixed by creating a bootloader for the archoses. (I don't know all of the technical details though). A bootloader should still fit in the available space for a very long time, there will be no need to cut features for archos rockbox. With kind regards, Bertrik There is that suggestion, or we could just drop support for HWCODEC completely... considering so few developers even have the AJBR (and fewer use it on a regular basis) what does keeping it around give us?
Re: Working on ATA: Less internal state, more asking
On 3 August 2012 23:51, Jonas Wielicki j.wieli...@sotecware.net wrote: Hi all, I'm (still *sigh*) hunting my SSD issues on iriver (as posted previously on the list[1][2]). I think it basically reduces to a “have you tried turning it off and on again” fix, because somehow the SSD seems to get stuck sometimes, a powercycle can mitigiate that in most of the cases. BUT, in the course I implemented some ATA parts which might come in handy with devices which are behaving weirdly, like going to sleep automatically, namely, I removed the internal rockbox state about the drive's power management state and added requests to get the current state instead (using CMD_CHECK_POWER_MODE). I realize that I'll probably have to keep some of my rockbox kernel patched to work more or less with that SSD (I really should try to return it, maybe it's just broken). But some of these changes may give rockbox advantages when dealing with buggy devices. Here is a summarized list of changes: * check the current power mode of the device before using it * explicitly wake it up, if needed * two levels of error handling: 1. do a hard reset 2. powercycle the IDE bus I wonder what, if any, of this is appreciated by rockbox so that I can wrap it up as one or more nice commits for review at gerrit. best regards, Jonas Wielicki [1]: http://www.rockbox.org/mail/archive/rockbox-dev-archive-2012-04/0010.shtml [2]: http://www.rockbox.org/mail/archive/rockbox-dev-archive-2012-05/0009.shtml Hi, Please put up your patches on gerrit and then we can figure out wether or not they are wanted in git (sounds like they are!). Thanks Jonathan
Re: Changing build/release terminology
On 28 March 2012 20:49, Torne Wuff to...@wolfpuppy.org.uk wrote: Hi folks, I would like to propose that we change our terminology used to describe the builds and releases, on the website and in Rockbox Utility. Currently we talk about release and current build a lot, and this confuses users over and over who don't understand the distinction, and I am inclined to agree with people who think this is confusing. I propose that we do our best to switch to the following terminology consistently in our written materials: 1) Stable release: e.g. stable release 3.10. 2) Development build: e.g. development build abc123g. 3) Archived development build: the daily snapshots Rockbox Utility can install. I think this would make the distinction much clearer, and it would also help to make it clear that the daily snapshots are not something most people should install (currently people often believe they are somehow more stable/reliable than the current builds, which is not the case). Please, reply with your thoughts. I'm not set on these particular terms, it's just an initial proposal. Thanks, -- Torne Wuff to...@wolfpuppy.org.uk Sounds good to me. Not sure if I like stable release though, what are other projects using? perhaps official release?
re FS#12625 - Sleep timer setting is broken
Hi, This wasnt caught when the sleep timer changes were commited, and its not really a big deal but it should be changed. IIUC the sleep timer is now broken into a few different settings: 1) set timer on boot 2) restart timer on keypress 3) start the timer with the chosen minutes value (arbitrarily set to 5min increments). Both the first two settings use the 3rd to know how long to start the timer for manually. The issue in the task is: 1) It isnt a normal setting so cant be added to the quickscreen and shortcuts menu (minor inconvinience) 2) cancelling the setting screen enables the timer anyway. - BAD! I don't understand why setting the sleep timer duration for boot should be linked to the sleep timer duration i want to set temporarily. (i.e the current system means unless I have a fixed.cfg it is impossible for me to set a 30min timer before i go to bed and have it turn on with a 90min timer). The duration should be a separate setting and the start sleep timer menu item should then be changed to start at off so pressing cancel will not enable a timer. In addition, extra handling for the sleep timer can be manually added to the shortcuts screen to start/stop a timer. Jonathan.
Re: re FS#12625 - Sleep timer setting is broken
On 28 March 2012 01:05, Nick Peskett rock...@peskett.co.uk wrote: Unfortunately this has been the case with the callback since 2007; http://svn.rockbox.org/viewvc.cgi/trunk/apps/menus/main_menu.c?view=diffpathrev=30777r1=12548r2=12549 As you move through the menu items the sleep timer continually gets reset, so when you cancel the menu item the last set remains. Yes, because it is using a callback for set_int() which really isnt necessary. I started this email last night after realising removing that callback and setting the initial value to off would break the other two settings. This is an easy fix. This is pretty much how it was in the original patch before many opinions on IRC and the ML stripped it down to the bare minimum changes acceptable to get it committed. http://www.rockbox.org/tracker/task/10849#comment40259 http://www.rockbox.org/mail/archive/rockbox-dev-archive-2011-08/0018.shtml I dont' see discussion in either of those links about stripping the options down, and anyway, they can probably be ignored anyway. Would you like to split it back to how it was? Jonathan
Re: librbcodec
On 28 February 2012 15:22, Sean Bartell wingedtachik...@gmail.com wrote: Hi, everyone, I realize I all but disappeared last summer when GSoC ended, and I'm sorry for leaving my work unfinished. I've spent some more time on it, and completed the patches that create librbcodec[0]. I think it's a reasonable way of separating the DSP, metadata, and codec code so they can be used by other software, but I haven't gotten much feedback on my overall approach. Does my code have the potential to be committed? Thanks, Sean Bartell [0] http://gerrit.rockbox.org/r/#/q/status:open+project:rockbox+branch:master+topic:rbcodec,n,z Hey! As always, the most likely way of getting patches pushed is hanging out in IRC. Jonathan
Reminder: Use Gerrit to submit patches/translation fixes, not flyspray
Hi all, Please remember we have mostly switched to gerrit for patch tracking and would like to only use flyspray for bugs. If your patches (translations especially) are submitted to gerrit it is much more likely they will be merged quickly (they can be done wih a single click from the website instead of requiring a checkout, download and apply the patch and push.) Useful links: http://gerrit.rockbox.org/ http://www.rockbox.org/wiki/UsingGit http://www.rockbox.org/wiki/UsingGit#Setting_up_Gerrit Finally, please add your full name to docs/CREDITS on the first patch you submit. Thanks Jonathan
Re: Reminder: Use Gerrit to submit patches/translation fixes, not flyspray
On 2 March 2012 09:56, Jonathan Gordon jdgo...@gmail.com wrote: Hi all, Please remember we have mostly switched to gerrit for patch tracking and would like to only use flyspray for bugs. If your patches (translations especially) are submitted to gerrit it is much more likely they will be merged quickly (they can be done wih a single click from the website instead of requiring a checkout, download and apply the patch and push.) Useful links: http://gerrit.rockbox.org/ http://www.rockbox.org/wiki/UsingGit http://www.rockbox.org/wiki/UsingGit#Setting_up_Gerrit Finally, please add your full name to docs/CREDITS on the first patch you submit. Thanks Jonathan I evidently forgot how translate.rockbox.org is used. Ignore this email (though still if you can, pushing through gerrit is easier/faster)/ Jonathan
Re: Call for testers for gerrit#120
On 26 February 2012 20:53, Yohan LEE-TIN-YIEN yohan.leetiny...@gmail.com wrote: Please download for your targets and use it for a bit to see if there is any issues with the LCD (redrawing, screen updates, etc). I've been testing on Sansa Clip+ for 2 days now, and basically what I experienced is : - everything works fine : menu, wps, rds, demos (all), games (only a few) - the menu, wps, rds, demos, games, etc. seems to me more responsive (compared to the daily build 2012-02-09 and 3.10 release) - While I plug the USB cable and hold the button in order to pass in charging battery mode, it is a little bit more difficult : it is more likely to ignore the button holding and goes in card reader mode. But I guess this is a side effect of a more responsive player. All in all, everything works fine for Sansa Clip+ with even an improvement of responsiveness for better and for worse (a little bit more tricky to hold button while pluging the usb cable). Yohan PS : Sorry if there are english mistakes, I'm french. Thanks :) I should have mentioned it at the start, this change is relatively minor and it will either work or it wont. So if you get any display at all after the bootloader it means the patch worked fine. I'm getting pretty confident that the patch (version 9) is commitable, but I havnt heard anyone testing the older targets without colour screens (except the ipod g2), so if you have one please give it a test.
Call for testers for gerrit#120
Hi all, http://gerrit.rockbox.org/r/#/c/120/ changes the way lcd drivers work internally and every single target needs testing (or at least a few to make sure it probably works for all of them). gevaerts has kindly built and hosted builds at http://rockbox.hostname.be/lcd-test/ Please download for your targets and use it for a bit to see if there is any issues with the LCD (redrawing, screen updates, etc). Thanks Jonathan
Re: having pluginlib-action handling more/all plugin.
On 6 January 2012 00:16, Jean-Louis Biasini jlbias...@gmail.com wrote: Hi, Best Regards, Jean-Louis Biasini The biggest issue with PLA is that quite a few of the plugins use the same button loop for their menus and their actual game button handling. That makes PLA unusable because buttons will work completly differently in those contexts. If you *really* want to do this then go ahead, but it is mostly boring and error prone work. Of couse, if you want to do this why not update individual plugins properly to updated APIs? i.e many don't use the multi-screen API at all (they use rb.lcd_put*() directly). There there is the whole Mr Someone's TODO list if you're looking for something else to work on :)
Re: ipod videos help
On 3 January 2012 15:12, austin galloway even...@gmail.com wrote: ok i have rockbox on my ipod video , and when i turn it on it has boot menu , with turnoff button , rockbox , then emor console , settings, how do i boot the original firmware ,ur directions on manual dont help because the apple logo doesnt show nor does either method work , can u please help me out , thanks 1) If you have a boot menu you arn't using rockbox's bootloader so obviously our instructions wont work. 2) This mailing list is for development discussion, use the user mailing list or IRC for support
Re: Moving sleep timer menu items/ restart timer on keypress
On 22 December 2011 00:31, Nick Peskett rock...@peskett.co.uk wrote: The root of what I'd like to change; only clock related options having a stonking great clock embedded at top of the menu, all non-RTC options having a permanent home. yeah, the system settings submenu :)
Re: HWCODEC
On 19 December 2011 04:39, Thomas Martitz ku...@rockbox.org wrote: Am 15.12.2011 00:21, schrieb Mike Giacomelli: The alternative I think is to keep both together indefinitely while accepting that people may not want to upgrade from what they're already running. I think this is a bad way forward because in practice if people do not upgrade, then we have left HWCODEC essentially unmaintained. People would basically be stuck with whatever bugs were built into the release or build they decide to stay with. I agree with forking out HWCODEC into a legacy series. It has many advantages, while the only real advantage that keeping it in has (which is that new features also apply to the eldest targets) is nullified if the actual users don't upgrade. HWCODEC, which includes CPU_SH and CHARCELL, largely unmaintained. There's no active development, and it's basically only a PITA for the active devs. The devs with HWCODEC targets aren't the active devs. In the meantime people try hard just to keep the build table green. They do this without the ability to test changes, as people with hwcodec targets are often not reachable. All this effort is useless if the users don't even upgrade. I wouldn't want to force useless effort onto devs, possibly lowering the creativity or rate of contributions. Another advantage not mentioned yet is that getting HWCODEC out allows for a big cleanup in the code base. Lots of the oldest and ugliest code is only for one (or more) of the aforementioned characteristics (hwcodec,sh,charcell). I see little sense in trying to support (yes, we currently don't really support it) hwcodec further. Removing the maintenance burden seems much more appealing. Best regards. This pretty much sounds like a given, so once we move to git we create a fork branch and start stripping crud out :)
Re: the preset files
On 17 December 2011 15:57, Scott Berry scottbb1...@gmail.com wrote: Hey all devs, I am going to begin work on the fm presets. A couple of questions is there any way that there could be a branch made for just uploading presets like you do the actual code and also is there any way we could automate the zip process so that maybe once a month it actually would zip up the presets automatically and place them on the correct spot on the wiki? Scott Berry Loving Rockbox for about four years now The wiki is where they should go. There is no need to put them in the svn repo as it would just slow down people contributing (see the language updates waiting in flyspray as proof)
Re: dreamlayers: r31296 - in trunk/apps: . metadata
This is exactly what we don't want to do! On 16 December 2011 07:58, mai...@svn.rockbox.org wrote: Date: 2011-12-15 21:58:14 +0100 (Thu, 15 Dec 2011) New Revision: 31296 Log Message: Add conditionals for functions only needed on SWCODEC targets. Modified: trunk/apps/metadata/id3tags.c trunk/apps/metadata/metadata_parsers.h trunk/apps/playlist.c trunk/apps/playlist.h trunk/apps/talk.c trunk/apps/talk.h Modified: trunk/apps/metadata/id3tags.c === --- trunk/apps/metadata/id3tags.c 2011-12-15 20:48:15 UTC (rev 31295) +++ trunk/apps/metadata/id3tags.c 2011-12-15 20:58:14 UTC (rev 31296) @@ -92,6 +92,9 @@ Synthpop }; +#if CONFIG_CODEC != SWCODEC +static +#endif char* id3_get_num_genre(unsigned int genre_num) { if (genre_num ARRAYLEN(genres)) Modified: trunk/apps/metadata/metadata_parsers.h === --- trunk/apps/metadata/metadata_parsers.h 2011-12-15 20:48:15 UTC (rev 31295) +++ trunk/apps/metadata/metadata_parsers.h 2011-12-15 20:58:14 UTC (rev 31296) @@ -19,11 +19,14 @@ * / +#if CONFIG_CODEC == SWCODEC char* id3_get_num_genre(unsigned int genre_num); +#endif int getid3v2len(int fd); bool setid3v1title(int fd, struct mp3entry *entry); void setid3v2title(int fd, struct mp3entry *entry); bool get_mp3_metadata(int fd, struct mp3entry* id3); +#if CONFIG_CODEC == SWCODEC bool get_adx_metadata(int fd, struct mp3entry* id3); bool get_aiff_metadata(int fd, struct mp3entry* id3); bool get_flac_metadata(int fd, struct mp3entry* id3); @@ -53,3 +56,4 @@ bool get_sgc_metadata(int fd, struct mp3entry* id3); bool get_vgm_metadata(int fd, struct mp3entry* id3); bool get_kss_metadata(int fd, struct mp3entry* id3); +#endif /* CONFIG_CODEC == SWCODEC */ Modified: trunk/apps/playlist.c === --- trunk/apps/playlist.c 2011-12-15 20:48:15 UTC (rev 31295) +++ trunk/apps/playlist.c 2011-12-15 20:58:14 UTC (rev 31296) @@ -1068,6 +1068,7 @@ return steps; } +#if CONFIG_CODEC == SWCODEC /* Marks the index of the track to be skipped that is steps away from * current playing track. */ @@ -1089,6 +1090,7 @@ playlist-indices[index] |= PLAYLIST_SKIPPED; } +#endif /* CONFIG_CODEC == SWCODEC */ /* * returns the index of the track that is steps away from current playing @@ -2638,6 +2640,7 @@ return index; } +#if CONFIG_CODEC == SWCODEC /* try playing next or previous folder */ bool playlist_next_dir(int direction) { @@ -2647,6 +2650,7 @@ return create_and_play_dir(direction, false) = 0; } +#endif /* CONFIG_CODEC == SWCODEC */ /* Get resume info for current playing song. If return value is -1 then settings shouldn't be saved. */ Modified: trunk/apps/playlist.h === --- trunk/apps/playlist.h 2011-12-15 20:48:15 UTC (rev 31295) +++ trunk/apps/playlist.h 2011-12-15 20:58:14 UTC (rev 31296) @@ -134,7 +134,9 @@ bool playlist_check(int steps); const char *playlist_peek(int steps, char* buf, size_t buf_size); int playlist_next(int steps); +#if CONFIG_CODEC == SWCODEC bool playlist_next_dir(int direction); +#endif int playlist_get_resume_info(int *resume_index); int playlist_update_resume_info(const struct mp3entry* id3); int playlist_get_display_index(void); @@ -158,7 +160,9 @@ bool recurse); int playlist_insert_playlist(struct playlist_info* playlist, const char *filename, int position, bool queue); +#if CONFIG_CODEC == SWCODEC void playlist_skip_entry(struct playlist_info *playlist, int steps); +#endif int playlist_delete(struct playlist_info* playlist, int index); int playlist_move(struct playlist_info* playlist, int index, int new_index); int playlist_randomise(struct playlist_info* playlist, unsigned int seed, Modified: trunk/apps/talk.c === --- trunk/apps/talk.c 2011-12-15 20:48:15 UTC (rev 31295) +++ trunk/apps/talk.c 2011-12-15 20:58:14 UTC (rev 31296) @@ -957,6 +957,7 @@ return 0; } +#if CONFIG_CODEC == SWCODEC /* Play a directory's .talk thumbnail, fallback to spelling the filename, or go straight to spelling depending on settings. */ int talk_dir_or_spell(const char* dirname, @@ -973,6 +974,7 @@ return talk_spell_basename(dirname, prefix_ids, enqueue); return 0; } +#endif /* say a numeric value, this word ordering works for english, but not necessarily for other languages (e.g. german) */ Modified: trunk/apps/talk.h === ---
Re: HWCODEC
On 15 December 2011 10:58, Rafaël Carré fun...@videolan.org wrote: Hello, Le Wed, 14 Dec 2011 18:21:42 -0500, Mike Giacomelli giac2...@hotmail.com a écrit : Advantages: ***Greatly simplify large parts of the code for SWCODEC targets (see JdGordon's forum posts in rockbox general) So, my issue isnt so much HWCODEC but CHARCELL support (which just happens to be HWCODEC). (also low ram targets). As I said in the forum post, all the ui customisability could be easily be removed in a low-mem config (which could even be done in svn if that is wanted). The topic really should be discussing deprecating/dropping LCD_CHARCELL, HWCODEC engine and what to do with low mem targets. Jonathan
Re: HWCODEC
On 15 December 2011 12:50, Boris Gjenero boris.gjen...@gmail.com wrote: Apparently, there are few HWCODEC users, and I'm not sure if any developers regularly use current builds on HWCODEC targets. Because of that, there isn't much focus on improving things for HWCODEC. On this point, I remember there was a HWCODEC related bug (I think involving folder advance in the wps) which went unnoticed for something like 6 months because of the lack of users *and developers* regularly using svn. Also, quite often in IRC there is a call to get HWCODEC testing done but absolutly no responses. (Admitadly this is an issue with alot of targets, but there are far fewer HWCODEC targets in the active dev group than most other targets).
Re: 3.10: Shut down menu entry
On 30 November 2011 07:23, Thomas Jarosch t...@simonv.com wrote: Hi, I've just given rockbox 3.10RC0 a spin and I really like the way it improved on touchscreen targets like the Nokia N900. This is probably thanks to kugel's list spacer patch. One small thing lacking for a nice out-of-the-box experience is a Shut down root menu entry. Support for that is already in rockbox and built if the target is an Archos Player. [Saint] posted a patch to enable the Shut down menu entry for all targets: http://pastebin.com/0dTBEnd4 Any chance we can squeeze something like this in for 3.10 or is it already too late in the RC phase? Do we want this for all targets or just for RaaA? Cheers, Thomas This seems to be one of those annoying arguments which goes on forever because noone can really agree on it. I don't see the need for a shutdown menu item, but I know people swear by it, so if it went in the System menu (even though I'd prefer that dissapeared) or we made it possible to go in the shortcut menu I'd be happy.
Re: Context and action for mapping keys
Please don't add special actions for a single device. We try to make all rockbox targets consistant with eachother so anyone picking up an ipod immediately knows how to use it if they are coming from a clip. Not only that, I'm very suspicious about how you plan on getting all the standard buttons in with a keypad which doesnt appear any less limited than the h300? On 28 November 2011 19:33, Jean-Louis Biasini jlbias...@gmail.com wrote: I would like: - some WPS to Database action (we have one to browser, one to playlist) The keymap is the wrong place for this. If you want to provide wps-file and wps-database it should be a user configurable option for all targets. didn't find one... - Context for playlist and action for deleting item in it that could be directly mapped to a device Use the ACTION_HOTKEY action for that. Jonathan
Re: Updating the theme site
On 28 November 2011 11:44, Mike Giacomelli giac2...@hotmail.com wrote: Alright does someone want to help me with that? Zip is 96x96 color. Fuze+ is the same gigabeat (320 x 240). Mike Done, though we need target images for the table
Re: FS#5111 - ipod piezo
On 16 November 2011 18:37, Thomas Martitz ku...@rockbox.org wrote: Am 16.11.2011 05:33, schrieb Jonathan Gordon: Hey all, Does anyone know why http://www.rockbox.org/tracker/task/5111 is not in svn? Jonathan Last time I looked I complained that it didn't integrate with the software keyclick. What's the situation there now? According to your comments you include an a change which should better be a separate task (don't repeat clicks on button repeats). Best regards. The task has been open for 5 years, so I'm obviously not asking about changes in the last month. Is there any reason the piezo driver part has not been accepted so far?
FS#5111 - ipod piezo
Hey all, Does anyone know why http://www.rockbox.org/tracker/task/5111 is not in svn? Jonathan
Re: Working towards skin engine 2.0 (includes RFC on code!)
On 14 November 2011 01:09, Frank Gevaerts fr...@gevaerts.be wrote: On Wed, Nov 09, 2011 at 12:17:02AM +1100, Jonathan Gordon wrote: I'm not entirely sure what to do about the conversion macros. I am going to keep the OFFSETTYPE() one (though open to a better name) because it is helpful). Is it really? I can't say I'm convinced, If someone could come up with a way to make OFFSETTYPE() actually *do* something (a clever way to do type checking, maybe), I'd agree. The way it stands now it's just as easy to overlook as a comment when changing things, and *nothing* will complain (the thing given there doesn't even have to be a valid type...). At a minimum it is needed so the same maasive rework isnt needed in the theme editor. And yes, I've found it ery useful during development to know what exatly is being pointed to. I agree it would be 100x better if the compiler could actually warn if the type isnt correct but thats not really doable.
Re: Working towards skin engine 2.0 (includes RFC on code!)
On 8 November 2011 22:58, Thomas Martitz ku...@rockbox.org wrote: Am 07.11.2011 14:54, schrieb Magnus Holmgren: I agree that the macros are a bit long. Also, there are no _ chars to separate things, making them a little harder to read. What about changing SKINOFFSETTOPTR to SKIN_TO_POINTER and PTRTOSKINOFFSET to SKIN_TO_OFFSET? Not much shorter perhaps, but easier to read. And the from part isn't that important, IMHO. Apart from the lengthy names (perhaps SKIN_2OFF, SKIN_2PTR?), it looks like the first parameter is exclusively skin_buffer. If that's true, can't it be omitted somehow? Best regards. skin_buffer is a placeholder. In all likelihood this will end up being different for each skin (usually core_get_data(data-buffer)).
Re: Working towards skin engine 2.0 (includes RFC on code!)
On 9 November 2011 01:06, Magnus Holmgren magnus...@gmail.com wrote: On Tue, Nov 8, 2011 at 14:17, Jonathan Gordon jdgo...@gmail.com wrote: Well, I honestly didnt think it would be this simple! Attached is the changes removing all dynamic pointers from the skin engine! One question: Since SKINOFFSETTOPTR and PTRTOSKINOFFSET both include a conditional and are used a lot, what happens with binsize? :) -- Magnus It really doesnt matter, they could add 1KB and we'd still be ~17KB better off on most targets. The skin buffer currently over allocates for cabbie by nearly 20KN on some targets.
Re: Working towards skin engine 2.0 (includes RFC on code!)
On 9 November 2011 01:12, Michael Chicoine mc2...@gmail.com wrote: On Tue, Nov 8, 2011 at 7:17 AM, Jonathan Gordon jdgo...@gmail.com wrote: Testing is pretty much: Apply and run your themes. If it works there should be no difference at all. A quick test on e200v1 with cabbiev2 (r30933) shows the following issues: 1. FM Radio screen is blank (except backdrop and statusbar). After settings clear, it does not even display the No settings found. Autoscan? message. 2. WPS displays incorrect icons: playmode always shows stop icon, hold icon shows locked when locked or unlocked, repeat mode shows repeat-all in all repeat modes. Crap! I've been testing with the video sim, it should all work everywhere :/ thanks for testing
Re: Working towards skin engine 2.0 (includes RFC on code!)
Test builds are available at http://jdgordon.info/rockbox/skintestbuilds/output/ On 9 November 2011 09:01, Jonathan Gordon jdgo...@gmail.com wrote: On 9 November 2011 01:12, Michael Chicoine mc2...@gmail.com wrote: On Tue, Nov 8, 2011 at 7:17 AM, Jonathan Gordon jdgo...@gmail.com wrote: Testing is pretty much: Apply and run your themes. If it works there should be no difference at all. A quick test on e200v1 with cabbiev2 (r30933) shows the following issues: 1. FM Radio screen is blank (except backdrop and statusbar). After settings clear, it does not even display the No settings found. Autoscan? message. 2. WPS displays incorrect icons: playmode always shows stop icon, hold icon shows locked when locked or unlocked, repeat mode shows repeat-all in all repeat modes. Crap! I've been testing with the video sim, it should all work everywhere :/ thanks for testing
Branched for 3.10
Hi all, I've just created the 3.10 release branch. To check it out, run: svn co svn://svn.rockbox.org/rockbox/branches/v3_10 rockbox-3.10 This means that trunk is again free for regular development. However, please don't be afraid to concentrate on bugs for the time being, and maybe don't do invasive and reasonably minor things like big whitespace cleanups yet, as they could make backporting fixes more difficult than they need to be. Let's fix as many bugs as we can during the next week!
Working towards skin engine 2.0 (includes RFC on code!)
Hi all, The only long term solution to the issue of the skin buffer size is to completly redo how the skin engine uses its buffer, to that end I have just started the rather large task of replacing every pointer in it to an offset into the (currently) shared buffer which is then turned back into a pointer when it is being used. One annoyance with the current skin engine code is that all skins are linked together with the single buffer, this means that if any skins need reloading (user request to change theme perhaps) all of them need to be reloaded. So one of the goals with this new work is to split that up so each skin manages its own buflib handle, allowing skins to be un/loaded on demand (there is no point loading the fm skin and wasting buffer space if the user never actualy loads the fm screen for example.) The plan is to load the skin into the plugin buffer (and buflib handles for images as done now), then do a memcpy() of the loaded skin into a new buflib handle of the exact size needed for the skin. This means people wanting boring themes don't limit people wanting Over The Top themes (and no extra ram is wasted). Using offsets to do this means there is also no need to manage pointers when the handle moves around. Another ultimate goal is to draw more of the UI using the skin engine, specifically screens which are currently hard coded for different screen sizes (the timedate screen is an obvious candidate), this is very far down the track though so we'll see if that ever actually happens. The attached patch is the very start of this work. I'm really writing this email because before I get too far in I want to make sure the macros are going to be acceptable. Three macros and a typedef have been added. SKINOFFSETTOPTR() and PTRTOSKINOFFSET() convert between the offset and the real pointer. I originally wanted to put the type in the first macro and use that instead of void* but not sure if that is really needed. the 3rd macro OFFSETTYPE() is added because I realised the structs used by the various tags will become very confusing if all the members are skinoffset (or long) types instead of the pointer type which the data actually contains. So I added this macro to hopefully help document the code a bit. Are these going to cause people to grimace? Is there a better way to do what they are doing? This is going to be a rather massive diff so if these macros are unacceptable I really want to know now rather than once it is all done. Cheers Jonathan P.S This is on my skinengine_to_offsets github branch https://github.com/jdgordon/rockbox/tree/skinengine_to_offsets and will likely be pushed upstream as a single patch when it is done, but would anyone like to help out? :D P.P.S If you have a patch that touches apps/gui/skin_engine/* please talk to me on IRC before committing so we dont cause extra work for eachother. diff --git a/apps/gui/skin_engine/skin_display.c b/apps/gui/skin_engine/skin_display.c index 95e4310..a352252 100644 --- a/apps/gui/skin_engine/skin_display.c +++ b/apps/gui/skin_engine/skin_display.c @@ -406,7 +406,7 @@ void wps_display_images(struct gui_wps *gwps, struct viewport* vp) { wps_draw_image(gwps, img, img-display); } -else if (img-always_display img-vp == vp) +else if (img-always_display SKINOFFSETTOPTR(skin_buffer, img-vp) == vp) { wps_draw_image(gwps, img, 0); } diff --git a/apps/gui/skin_engine/skin_engine.c b/apps/gui/skin_engine/skin_engine.c index bd875fe..8a6152f 100644 --- a/apps/gui/skin_engine/skin_engine.c +++ b/apps/gui/skin_engine/skin_engine.c @@ -48,7 +48,7 @@ void theme_init_buffer(void) skins_initialising = false; } #else -static char skin_buffer[SKIN_BUFFER_SIZE]; +char skin_buffer[SKIN_BUFFER_SIZE]; void theme_init_buffer(void) { skins_initialising = false; diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c index 1557783..bf2179c 100644 --- a/apps/gui/skin_engine/skin_parser.c +++ b/apps/gui/skin_engine/skin_parser.c @@ -168,7 +168,7 @@ void *skin_find_item(const char *label, enum skin_find_what what, #ifdef HAVE_LCD_BITMAP case SKIN_FIND_IMAGE: ret = list.linkedlist-token-value.data; -itemlabel = ((struct gui_img *)ret)-label; +itemlabel = SKINOFFSETTOPTR(skin_buffer, ((struct gui_img *)ret)-label); break; #endif #ifdef HAVE_TOUCHSCREEN @@ -295,9 +295,9 @@ static int parse_image_display(struct skin_element *element, { return WPS_ERROR_INVALID_PARAM; } -id-label = label; +id-label = PTRTOSKINOFFSET(skin_buffer, label); id-offset = 0; -id-token = NULL; +id-token = PTRTOSKINOFFSET(skin_buffer, NULL); if (img-using_preloaded_icons) { token-type = SKIN_TOKEN_IMAGE_DISPLAY_LISTICON; @@ -306,7 +306,7 @@ static int parse_image_display(struct skin_element *element,
Re: 3.10 (was Re: saratoga: r30837 - in trunk: apps apps/lang manual/configure_rockbox)
On 7 November 2011 01:47, Thomas Martitz ku...@rockbox.org wrote: Am 05.11.2011 12:16, schrieb Alex Parker: Excellent, thanks very much :) Looks like both issues are resolved, so we should be ready to freeze. Best regards. In Alex's absence (as the person who's been pushing the last few release) can we decide in IRC tonight if we branch and release?
FS#12251 - user shortcuts in the main menu
Hi all, So I've been working on this patch for a while, and when I started I didnt really intend on pushing for this to get committed, but now it is pretty much finished I feel that it would be a shame if it isnt as it fixes pretty much everyones complaint with the menus and quickscreen (and adds a bunch of extra niceness for the hell of it :) ) What this does it add a new menu item Shortcuts to the end of the menu which is populated from /.rockbox/shortcuts.txt. I've made it very flexible and tried to add anything which anyone would find useable. The following are the available shortcut types: * run a file (same as navigating to the file in the browser and clicking select * open the browser at a file/folder (at or in a folder. /Music/foo will put you in /Music with foo selected while /Music/foo/ will put oyu in /Music/foo with the first file selected) * show the current playlist context menu on a file or folder (direct access to that menu for your favorite music) * open the setting chooser for any setting which can go on the quickscreen (i.e 99% of the useful ones) * open any of the debug menu screens The only type which I really want but cant add is a shortcut into the database tree, but the usual issues with database prevents that :/ If there are any other shortcuts people might want I'm open to adding them. You can also choose the text and icon of each item, and add spacer items so help group your shortcuts (if that makes it easier for you). I've taken over the Add to shortcuts context menu item in the browser (adds a browse type for the file/folder) and added the context menu item to the settings and the debug screens menus so they can be easily added. I'd like to make a plugin to more easily modify the shortcut file but it isnt really necessary. This combined with http://www.rockbox.org/tracker/task/12361 (which makes config loading much faster) should easily solve any problems anyone has with the quickscreen or menu. So try it out and let me know what you think.
Re: FS#12251 - user shortcuts in the main menu
On 1 November 2011 22:39, Peter D'Hoye peter.dh...@telenet.be wrote: What this does it add a new menu item Shortcuts to the end of the menu which is populated from /.rockbox/shortcuts.txt. I've made it very flexible and tried to add anything which anyone would find useable. The following are the available shortcut types: * run a file (same as navigating to the file in the browser and clicking select * open the browser at a file/folder (at or in a folder. /Music/foo will put you in /Music with foo selected while /Music/foo/ will put oyu in /Music/foo with the first file selected) * show the current playlist context menu on a file or folder (direct access to that menu for your favorite music) * open the setting chooser for any setting which can go on the quickscreen (i.e 99% of the useful ones) * open any of the debug menu screens I hope/assume your patch removes the existing shortcuts system (which is for folders only)? I think having two systems might be confusing /P I didnt touch the existing one which is a plugin. Thats a topic for another discussion. The plugin based one has the advatange of being able t handle more than one shortcuts file which this patch cant do.
Re: 3.10 (was Re: saratoga: r30837 - in trunk: apps apps/lang manual/configure_rockbox)
On 27 October 2011 19:33, Björn Stenberg bj...@haxx.se wrote: Thomas Martitz wrote: FS#12279 - Sansa Clip+: Music playback is returned to the head when wps is changed since r30486 The last one isn't easily fixable (according to jhMikeS), and I also don't consider it release critical (though it's technically a regression indeed). The bug in the title might be non-critical, but the comments contain reports of EOM panics. -- Björn Considering that task hasnt been touched in a month, and that on my clip+ it works fine (not using svn though so possibly a different fix fixed it), I think that might be safe to close, unles someone can confirm it is still there)
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
The patch earlier had a compile error, this one should be good to go. On 26 October 2011 01:03, Jonathan Gordon jdgo...@gmail.com wrote: On 26 October 2011 00:52, Thomas Martitz ku...@rockbox.org wrote: Am 25.10.2011 15:40, schrieb Jonathan Gordon: There is an argument that setuifont() must call lcd_setfont(), but if setfont() sets global_status that would be wrong. Proof of that is what should global_status.font_id[SCREEN_MAIN] be set to after a call to screens[SCREEN_MAIN].setfont(FONT_UI) ? I thought FONT_UI doesn't exist anymore, because the id isn't constant anymore. There comes my confusion from. Well, some constant is needed for everything to be able to ask for the setting font, and FONT_UI is annoyingly everywhere already. On a slightly related note: Why is the font id in global_status, and not e.g. a member of the screen_access api? IMO it would make more sense in the latter seeing that font getters/setters are also there. There is really no difference, global_status was a convenient place to put it, putting it in the screen_access struct *might* mean it will be in the cache when accessed, perhaps. This is very hair-splitting, though if whoever gets to commiting wants to do that I have no objection. diff --git a/apps/filetree.c b/apps/filetree.c index 2407be9..d33b823 100644 --- a/apps/filetree.c +++ b/apps/filetree.c @@ -424,13 +424,10 @@ static void ft_load_font(char *file) set_file(file, (char *)global_settings.font_file, MAX_FILENAME); #endif splash(0, ID2P(LANG_WAIT)); -current_font_id = global_status.font_id[screen]; +current_font_id = screens[screen].getuifont(); if (current_font_id = 0) font_unload(current_font_id); -current_font_id = font_load(file); -if(screen==SCREEN_MAIN) -font_set_ui(current_font_id); -global_status.font_id[screen] = current_font_id; +screens[screen].setuifont(font_load(file)); viewportmanager_theme_changed(THEME_UI_VIEWPORT); } #endif diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c index a9689a8..c6632b86 100644 --- a/apps/gui/skin_engine/skin_parser.c +++ b/apps/gui/skin_engine/skin_parser.c @@ -249,7 +249,7 @@ static int parse_statusbar_tags(struct skin_element* element, /* viewport_set_defaults() sets the font to FONT_UI+curr_screen. * This parser requires font 1 to always be the UI font, * so force it back to FONT_UI and handle the screen number at the end */ -default_vp-vp.font = FONT_UI; +default_vp-vp.font = 1; #endif } return 0; @@ -1645,7 +1645,7 @@ static bool skin_load_fonts(struct wps_data *data) font_id = skin_vp-parsed_fontid; if (font_id == 1) { /* the usual case - built-in fonts */ -vp-font = global_status.font_id[curr_screen]; +vp-font = screens[curr_screen].getuifont(); continue; } else if (font_id = 0) diff --git a/apps/gui/skin_engine/skin_render.c b/apps/gui/skin_engine/skin_render.c index 369bd46..4d41a6f 100644 --- a/apps/gui/skin_engine/skin_render.c +++ b/apps/gui/skin_engine/skin_render.c @@ -658,7 +658,7 @@ void skin_render_viewport(struct skin_element* viewport, struct gui_wps *gwps, /* fix font ID's */ if (skin_viewport-parsed_fontid == 1) -skin_viewport-vp.font = global_status.font_id[display-screen_type]; +skin_viewport-vp.font = display-getuifont(); #endif while (line) diff --git a/apps/gui/statusbar-skinned.c b/apps/gui/statusbar-skinned.c index 7850e7c..f79672c 100644 --- a/apps/gui/statusbar-skinned.c +++ b/apps/gui/statusbar-skinned.c @@ -128,7 +128,7 @@ struct viewport *sb_skin_get_info_vp(enum screen_type screen) if (!vp) return NULL; if (vp-parsed_fontid == 1) -vp-vp.font = global_status.font_id[screen]; +vp-vp.font = screens[screen].getuifont(); return vp-vp; } diff --git a/apps/gui/usb_screen.c b/apps/gui/usb_screen.c index 06770b1..2b7d472 100644 --- a/apps/gui/usb_screen.c +++ b/apps/gui/usb_screen.c @@ -265,8 +265,8 @@ void gui_usb_screen_run(bool early_usb) #ifdef HAVE_LCD_BITMAP FOR_NB_SCREENS(i) { -font_unload(global_status.font_id[i]); -global_status.font_id[i] = -1; +font_unload(screens[i].getuifont()); +screens[i].setuifont(FONT_SYSFIXED); } skin_unload_all(); #endif diff --git a/apps/gui/viewport.c b/apps/gui/viewport.c index c5e4427..b17f765 100644 --- a/apps/gui/viewport.c +++ b/apps/gui/viewport.c @@ -317,7 +317,7 @@ void viewport_set_fullscreen(struct viewport *vp, #ifndef __PCTOOL__ set_default_align_flags(vp); #endif -vp-font = global_status.font_id[screen]; +vp-font = screens[screen].getuifont(); vp-line_height = font_get(vp-font)-height; vp-drawmode = DRMODE_SOLID; #if LCD_DEPTH 1 diff --git a/apps/plugins/bubbles.c b/apps/plugins/bubbles.c index
re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
On 25 October 2011 17:35, Thomas Martitz ku...@rockbox.org wrote: Plugins can use the screen_access api. Ideally those should be using a custom viewport if they want a custom font, right? Any plugins which are not using the screen acess api are probably so old that yes, they dont use a custom font anyway (and wont be modified to any time soon) One question about the patch though: Why separate setfont() and setuifont()? I imagine one could be sufficient, but perhaps I'm missing something. Best regards. There are a few issues here: The biggest issue is that the lcd API is full of 10 years of evolution. First the multiscreen api came in, and then viewports, both adding different bits of cruft. (not necessarily bad cruft). lcd_* functions *mostly* work on the current viewport (lcd_setfont() included), and some work on a specific viewport, and others work on the actual display as a whole. lcd_setfont() changes the current viewports font number. Should this be removed completly? The *only* time the caller doesnt have access to the viewport struct is with the default viewport (which we are trying to slowly get away from, and this should only happen in plugins now, so no, we cant remove it just yet). screen_access-setfont() is a wrapper for lcd_setfont() to make sure FONT_UI gets set correctly. screen_access-setuifont() does not call lcd_setfont() and only sets global_settings.font_ui[screen]. screen_acces-getuifont() just returns the global_settings.font_ui[screen] value (and there is no lcd_getfont()). Might be worthwhile also to check if font_get(vp-font)-height calls can be replaced with screen-getcharheight(). I admit I forgot about that one before Björn mentioned it and used the former too. getcharheight() is just a wrapper around font_get(lcd_getfont())-height so it only works on the current viewports current font, which isnt necessarily what the caller wants. even if it is, we want get get rid of lcd_getfont() anyway so at a minimum this wrapper needs replacing.
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
On 25 October 2011 19:49, Thomas Martitz ku...@rockbox.org wrote: Am 25.10.2011 09:36, schrieb Jonathan Gordon: One question about the patch though: Why separate setfont() and setuifont()? I imagine one could be sufficient, but perhaps I'm missing something. Best regards. There are a few issues here: The biggest issue is that the lcd API is full of 10 years of evolution. First the multiscreen api came in, and then viewports, both adding different bits of cruft. (not necessarily bad cruft). lcd_* functions *mostly* work on the current viewport (lcd_setfont() included), and some work on a specific viewport, and others work on the actual display as a whole. lcd_setfont() changes the current viewports font number. Should this be removed completly? The *only* time the caller doesnt have access to the viewport struct is with the default viewport (which we are trying to slowly get away from, and this should only happen in plugins now, so no, we cant remove it just yet). screen_access-setfont() is a wrapper for lcd_setfont() to make sure FONT_UI gets set correctly. screen_access-setuifont() does not call lcd_setfont() and only sets global_settings.font_ui[screen]. screen_acces-getuifont() just returns the global_settings.font_ui[screen] value (and there is no lcd_getfont()). Okay, I understand the difference. But why does there need to be one? In your patch you call them both most of the time. I understand setfont() sets the current (i.e. default vp) font, and setuifont() sets the id which was previously constant (FONT_UI). FONT_UI is the alias for the screen's default font which is in fact simply the font of the default vp. So, from my understanding you want to do both calls at the same time. Unless there is a reason to separate these steps which I'm missing, I suggest the behavior to be merged in setfont(). Best regards. setuifont() should only be called when actually loading a new font from disk. I didnt really want to add it at all so its just there for more completeness.
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
On 25 October 2011 16:50, Jonathan Gordon jdgo...@gmail.com wrote: Attached is 90% of the work to do this somewhat more nicely. The remaining work is to check each plugin and make sure they don't use lcd_setfont() directly (which a quick look shows alot do :( ) the quick fix for that is just put a wrapper in the plugin api for it like what has been added for the screen_access api. The plugin api already uses the screean access setfont helper so nothing more should need to be done... everything should just work (wishful thinking I know :p )
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
On 26 October 2011 00:52, Thomas Martitz ku...@rockbox.org wrote: Am 25.10.2011 15:40, schrieb Jonathan Gordon: There is an argument that setuifont() must call lcd_setfont(), but if setfont() sets global_status that would be wrong. Proof of that is what should global_status.font_id[SCREEN_MAIN] be set to after a call to screens[SCREEN_MAIN].setfont(FONT_UI) ? I thought FONT_UI doesn't exist anymore, because the id isn't constant anymore. There comes my confusion from. Well, some constant is needed for everything to be able to ask for the setting font, and FONT_UI is annoyingly everywhere already. On a slightly related note: Why is the font id in global_status, and not e.g. a member of the screen_access api? IMO it would make more sense in the latter seeing that font getters/setters are also there. There is really no difference, global_status was a convenient place to put it, putting it in the screen_access struct *might* mean it will be in the cache when accessed, perhaps. This is very hair-splitting, though if whoever gets to commiting wants to do that I have no objection.
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
On 25 October 2011 05:16, Fred Bauer fred.w.ba...@gmail.com wrote: If this is reverted, can it be delayed until after the release or until there is an alternative? (Jonathan said last night on IRC that he would work on it.) Several things were broken before r30826 and there are a considerable number of files that still expect font_get(FONT_UI) to provide the user selected font instead of the largest font index of the loaded fonts. Suppose a theme uses a small font with a very limited number of glyphs for a WPS numerical display and it happens to be the last font loaded by the skin parser. If all these files are not rewritten to use global_status.font_id[] the next display of FONT_UI is going to cause the player to saw through the hard disk because of all the cache misses. (There can be three disk seeks for every cache miss.) I didnt have time to work on this last night, hopefully I'll post a patch tonight If r30826 is reverted, all the ~40 files that depend on font_get(FONT_UI) should be updated to use global_status.font_id[]. FONT_UI should then be removed from font.h and considering the time that FONT_UI has been around, it would also be helpful to put a comment in the SVN commit and in font.h that FONT_UI is now deprecated so that contributors will not be publicly upbraided for trying to fix it when broken. Relax, noone is being publicly unbraided. And yes, a note abou FONT_UI being deprecated should have been done with the buflib commit Finally, bug hunting is not especially fun. What incentive does someone have to contribute to the project if all their ideas are promptly rejected without consideration? I'm aggravated. At this point, I really feel like I could write a plugin that cured cancer, AIDS, and hangovers and JD would reject it. Improve the v-keyboard? JD opposed. Allocate a sensible number of glyphs worth of font memory instead of over 1250 for an iPod Video (8818 for a Fuze!)? JD opposed. The same for skin fonts? JD accepted but then silently reverted. Save a glyphcache file per font? JD agreed with that...after he rejected the idea and I wrote an alternative. Now he's giving me grief because I didn't know about the unpublished deprecation of FONT_UI. Good times! I have no idea what you're talking about. Yes I initially had reservations with the glyph allocation patch, but you posted numbers and I shut up (actually I thought i replied to the tracker but apparently didn't). Bug fixes should never be done secretly, had you asked on IRC I would have said that your bugfix wasnt ideal. Actually thats probably a lie because I think the whole lifetime of this patch was when I was offline, hence me bringing it up on the mailing list. This message: [ Message body ] [ More options ] Related messages: [ Next message ] [ Previous message ] From: Jonathan Gordon jdgordy_at_gmail.com Date: Sun, 23 Oct 2011 16:31:50 +1100 Can this please be reverted? firmware/fonts.c should now know or care about the ui font at all, and post buflib fonts it doesnt. screen_access.c added a helper to set the font (screens[screen].set_font() ) which should be being used by the keyboard and lrcviewer. If those are still having issues then that helper might need more work. (actually it appears keyboard.c needs to use font_get() which has no helper to get global_status.font_id[screen]. Ideally a font_get_height() function be added which (to the screen api) to get the correct font height seen as that is what most calls to font_get() actually care about. On 23 October 2011 04:13, mailer_at_svn.rockbox.org wrote: Date: 2011-10-22 19:13:33 +0200 (Sat, 22 Oct 2011) New Revision: 30826 Log Message: Add functions font_set_ui() and font_get_ui(). The font returned by FONT_UI used to be fixed at zero but since buflib-fonts (r30589) can be different, depending on the order of loads and unloads. Fixes broken behavoir in virtual keyboard (FS#12336), lyrics player (FS#12306), and hopefully, FS#12337 ... ___ rockbox-cvs mailing list rockbox-cvs_at_cool.haxx.se http://cool.haxx.se/cgi-bin/mailman/listinfo/rockbox-cvs Received on 2011-10-23 -- Fred W. Bauer fred.w.ba...@gmail.com
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
Attached is 90% of the work to do this somewhat more nicely. The remaining work is to check each plugin and make sure they don't use lcd_setfont() directly (which a quick look shows alot do :( ) the quick fix for that is just put a wrapper in the plugin api for it like what has been added for the screen_access api. On 23 October 2011 16:31, Jonathan Gordon jdgo...@gmail.com wrote: Can this please be reverted? firmware/fonts.c should now know or care about the ui font at all, and post buflib fonts it doesnt. screen_access.c added a helper to set the font (screens[screen].set_font() ) which should be being used by the keyboard and lrcviewer. If those are still having issues then that helper might need more work. (actually it appears keyboard.c needs to use font_get() which has no helper to get global_status.font_id[screen]. Ideally a font_get_height() function be added which (to the screen api) to get the correct font height seen as that is what most calls to font_get() actually care about. On 23 October 2011 04:13, mai...@svn.rockbox.org wrote: Date: 2011-10-22 19:13:33 +0200 (Sat, 22 Oct 2011) New Revision: 30826 Log Message: Add functions font_set_ui() and font_get_ui(). The font returned by FONT_UI used to be fixed at zero but since buflib-fonts (r30589) can be different, depending on the order of loads and unloads. Fixes broken behavoir in virtual keyboard (FS#12336), lyrics player (FS#12306), and hopefully, FS#12337 Modified: trunk/apps/filetree.c trunk/apps/gui/list.c trunk/apps/settings.c trunk/firmware/export/font.h trunk/firmware/font.c Modified: trunk/apps/filetree.c === --- trunk/apps/filetree.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/apps/filetree.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -427,7 +427,10 @@ current_font_id = global_status.font_id[screen]; if (current_font_id = 0) font_unload(current_font_id); - global_status.font_id[screen] = font_load(file); + current_font_id = font_load(file); + if(screen==SCREEN_MAIN) + font_set_ui(current_font_id); + global_status.font_id[screen] = current_font_id; viewportmanager_theme_changed(THEME_UI_VIEWPORT); } #endif Modified: trunk/apps/gui/list.c === --- trunk/apps/gui/list.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/apps/gui/list.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -133,6 +133,7 @@ static int list_get_nb_lines(struct gui_synclist *list, enum screen_type screen) { struct viewport *vp = list-parent[screen]; + vp-line_height = font_get(vp-font)-height; int lines = skinlist_get_line_count(screen, list); if (lines 0) { Modified: trunk/apps/settings.c === --- trunk/apps/settings.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/apps/settings.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -886,6 +886,7 @@ if (global_status.font_id[SCREEN_MAIN] = 0) font_unload(global_status.font_id[SCREEN_MAIN]); rc = font_load(buf); + font_set_ui(rc); CHART2(font_load , global_settings.font_file); global_status.font_id[SCREEN_MAIN] = rc; lcd_setfont(rc); Modified: trunk/firmware/export/font.h === --- trunk/firmware/export/font.h 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/firmware/export/font.h 2011-10-22 17:13:33 UTC (rev 30826) @@ -55,7 +55,7 @@ /* SYSFONT, FONT_UI, FONT_UI_REMOTE + MAXUSERFONTS fonts in skins */ #define MAXFONTS (FONT_FIRSTUSERFONT + MAXUSERFONTS) -#define FONT_UI MAXFONTS +#define FONT_UI MAXUSERFONTS /* * .fnt loadable font file format definition @@ -124,6 +124,9 @@ void font_unload(int font_id); void font_unload_all(void); void font_lock(int font_id, bool lock); +/* set the default UI font */ +void font_set_ui(int font_id); +int font_get_ui(void); struct font* font_get(int font); Modified: trunk/firmware/font.c === --- trunk/firmware/font.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/firmware/font.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -88,6 +88,7 @@ }; static int buflib_allocations[MAXFONTS]; +static int font_ui = -1; static int cache_fd; static struct font* cache_pf; @@ -559,7 +560,7 @@ lock_font_handle(handle, false); buflib_allocations[font_id] = handle; - //printf(%s - [%d] - %d\n, path, font_id, *handle); + //printf(%s - [%d] - %d\n, path, font_id, handle); return font_id; /* success!*/ } int font_load(const char *path) @@ -616,30 +617,54 @@ /* * Return a pointer to an incore font structure
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
Can this please be reverted? firmware/fonts.c should now know or care about the ui font at all, and post buflib fonts it doesnt. screen_access.c added a helper to set the font (screens[screen].set_font() ) which should be being used by the keyboard and lrcviewer. If those are still having issues then that helper might need more work. (actually it appears keyboard.c needs to use font_get() which has no helper to get global_status.font_id[screen]. Ideally a font_get_height() function be added which (to the screen api) to get the correct font height seen as that is what most calls to font_get() actually care about. On 23 October 2011 04:13, mai...@svn.rockbox.org wrote: Date: 2011-10-22 19:13:33 +0200 (Sat, 22 Oct 2011) New Revision: 30826 Log Message: Add functions font_set_ui() and font_get_ui(). The font returned by FONT_UI used to be fixed at zero but since buflib-fonts (r30589) can be different, depending on the order of loads and unloads. Fixes broken behavoir in virtual keyboard (FS#12336), lyrics player (FS#12306), and hopefully, FS#12337 Modified: trunk/apps/filetree.c trunk/apps/gui/list.c trunk/apps/settings.c trunk/firmware/export/font.h trunk/firmware/font.c Modified: trunk/apps/filetree.c === --- trunk/apps/filetree.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/apps/filetree.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -427,7 +427,10 @@ current_font_id = global_status.font_id[screen]; if (current_font_id = 0) font_unload(current_font_id); - global_status.font_id[screen] = font_load(file); + current_font_id = font_load(file); + if(screen==SCREEN_MAIN) + font_set_ui(current_font_id); + global_status.font_id[screen] = current_font_id; viewportmanager_theme_changed(THEME_UI_VIEWPORT); } #endif Modified: trunk/apps/gui/list.c === --- trunk/apps/gui/list.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/apps/gui/list.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -133,6 +133,7 @@ static int list_get_nb_lines(struct gui_synclist *list, enum screen_type screen) { struct viewport *vp = list-parent[screen]; + vp-line_height = font_get(vp-font)-height; int lines = skinlist_get_line_count(screen, list); if (lines 0) { Modified: trunk/apps/settings.c === --- trunk/apps/settings.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/apps/settings.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -886,6 +886,7 @@ if (global_status.font_id[SCREEN_MAIN] = 0) font_unload(global_status.font_id[SCREEN_MAIN]); rc = font_load(buf); + font_set_ui(rc); CHART2(font_load , global_settings.font_file); global_status.font_id[SCREEN_MAIN] = rc; lcd_setfont(rc); Modified: trunk/firmware/export/font.h === --- trunk/firmware/export/font.h 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/firmware/export/font.h 2011-10-22 17:13:33 UTC (rev 30826) @@ -55,7 +55,7 @@ /* SYSFONT, FONT_UI, FONT_UI_REMOTE + MAXUSERFONTS fonts in skins */ #define MAXFONTS (FONT_FIRSTUSERFONT + MAXUSERFONTS) -#define FONT_UI MAXFONTS +#define FONT_UI MAXUSERFONTS /* * .fnt loadable font file format definition @@ -124,6 +124,9 @@ void font_unload(int font_id); void font_unload_all(void); void font_lock(int font_id, bool lock); +/* set the default UI font */ +void font_set_ui(int font_id); +int font_get_ui(void); struct font* font_get(int font); Modified: trunk/firmware/font.c === --- trunk/firmware/font.c 2011-10-22 15:28:09 UTC (rev 30825) +++ trunk/firmware/font.c 2011-10-22 17:13:33 UTC (rev 30826) @@ -88,6 +88,7 @@ }; static int buflib_allocations[MAXFONTS]; +static int font_ui = -1; static int cache_fd; static struct font* cache_pf; @@ -559,7 +560,7 @@ lock_font_handle(handle, false); buflib_allocations[font_id] = handle; - //printf(%s - [%d] - %d\n, path, font_id, *handle); + //printf(%s - [%d] - %d\n, path, font_id, handle); return font_id; /* success!*/ } int font_load(const char *path) @@ -616,30 +617,54 @@ /* * Return a pointer to an incore font structure. - * If the requested font isn't loaded/compiled-in, - * decrement the font number and try again. + * Return the requested font, font_ui, or sysfont */ -struct font* font_get(int font) +struct font* font_get(int font_id) { - struct font* pf; - if (font == FONT_UI) - font = MAXFONTS-1; - if (font = FONT_SYSFIXED) + struct buflib_alloc_data *alloc; + struct font *pf; + int
Re: Ladies and gentlemen, we have sound on HiFiMan HM-601.
On 18 October 2011 19:14, Francisco Vila paconet@gmail.com wrote: Good work. BTW I like this style of announcing, with ladies and gentlemen and a log of the first track played and so on. Gives an epic flavour to the whole thing. Thanks It is quite a tradition here :) http://www.rockbox.org/wiki/GentlemenMails
Re: Trimming the lcd api
On 13 October 2011 07:19, Björn Stenberg bj...@haxx.se wrote: However there is certainly a point in not requiring all arguments all the time, so two functions make sense. Since they are vararg enabled, I'd say a more suitable name than lcd_puts is lcd_printf and its extended companion lcd_xprintf. Sounds good.
Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot
On 13 October 2011 19:48, Thomas Martitz ku...@rockbox.org wrote: Am 13.10.2011 10:42, schrieb Thomas Martitz: Alright, thanks for the comments discussion. I'll commit part 1) (move TD to settings) and 3) (the actual sleep timer remake) and leave the System-About rename out for now. That is, in a few hours if there's no further objection. So one objection isnt enough? commit the sleep timer rework by all means, but dont move td unless the rest of the discusison is settled.
Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot
On 15 October 2011 20:53, Björn Stenberg bj...@haxx.se wrote: Jonathan Gordon wrote: So one objection isnt enough? commit the sleep timer rework by all means, but dont move td unless the rest of the discusison is settled. Your objection as I understood it was that the patch does less than you'd like, not that what it does is wrong. If that is a misunderstanding, could you please clarify your stance? -- Björn No, that's correct.
Re: My view on Rockbox for Android
On 10 October 2011 20:27, Thomas Martitz ku...@rockbox.org wrote: I'd like to add that it's worthwhile to invest into our UI code. We shouldn't forget that android isn't the only touch-enabled target. We should have something workable on other touch targets as well. HELLO! I don't think we've met. My name is Jonathan and I've been secretly working on the UI for the last year or so :) As of now the skin engine knows exactly which screen has focus (err of the list of 16 or so avilable) and with nothing more than an sbs I should be able to implement the context menu dialog Bjorn wants (with the exception of the touch feedback colouring). The entire UI is now skinnable to whatever the user wants with very fine grained control. And anything that it cant do can be added with a few lines in the skin engine draw code. I admit there are some unimplemented things which need fixing, but please don't go blaming rockbox's look on the lack of themeability. I've been slowly trying to move all GUI drawing from hard coded c to dynamic drawing with the skin engine. The remaining screens are pretty much the graphic EQ, colour chooser (no real point there), and the recording screen.
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 11 October 2011 08:45, Björn Stenberg bj...@haxx.se wrote: Thomas Martitz wrote: I agree it's less than ideal that there are two list implementations, and I see the skin engine eventually taking things over. But as of now there just are two implementations, and the classic lists aren't going to go away very soon. Plus the skinned lists have bugs and are imcomplete/not capable enough yet. Until that's worked out I'd still like to see my patch in for the meantime since it does improve usablity a lot _now_. Fine, implementing this in the classic list until the skinned list is ready to replace it sounds like a good plan to me. -- Björn Doing that gives little motivation for someone to finish off the missing bits of skinned list
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 9 October 2011 18:38, Jonathan Gordon jdgo...@gmail.com wrote: 30min of fiddling around, attached is the screenshot from the e200 sim and the patch to do it, notice there is no code needed anywhere outside of the skin engine (infact its only done there because that pulls in the generated .sbs if none is specified). No actual code needs changing. Notes about the screenshot: * I'm not sure why the images arent being displayed, that might be a skin engine bug, more likely PEBCAK. * Gradient isnt completly setup in that example Now, I haven't seen screenshots from kugels patch, but if it colours the whole line instead of just the text, then that can be fixed with minor changes to the skin engine (wanted to do that anyway). Though it requires the lcd driver get a draw more which is use the current pixels for the transparent pixels instead of the backdrop image) which IIUC amiconn has mentioned he wants to add for a while now. If kugels patch is the same as that screenshot then the above is still something which should be done :) (hardly a reason to not go by this implementation anyway). With a bit more fiddling this could be *easily* extended to *any* skinned list theme by just fiddling with the Lb() tag. Jonathan DAMMIT! pressed send before ataching the patch diff --git a/apps/gui/statusbar-skinned.c b/apps/gui/statusbar-skinned.c index ad9a391..d2b3a0d 100644 --- a/apps/gui/statusbar-skinned.c +++ b/apps/gui/statusbar-skinned.c @@ -192,7 +192,7 @@ void sb_skin_set_update_delay(int delay) */ char* sb_create_from_settings(enum screen_type screen) { -static char buf[128]; +static char buf[512]; char *ptr, *ptr2; int len, remaining = sizeof(buf); int bar_position = statusbar_position(screen); @@ -263,6 +263,21 @@ char* sb_create_from_settings(enum screen_type screen) len = snprintf(ptr, remaining, %%ax%%Vi(-,0,%d,-,%d,1)\n, y, height); } +remaining -= len; +ptr += len; +/* now do the skin list */ +int font_height = font_get(global_status.font_id[screen])-height; +int padding = 6; // get it from wherever +int icon_size = get_icon_width(screen); +len = snprintf(ptr, remaining, +%%V(0,0,0,0,0)\n /* %Lb() needs to be in a viewport */ +%%Lb(a,%d,%d)\n /* setup the list viewport rectangle */ +%%Vl(a,0,%d,%d,%d,-)\n%%xl(i,__list_icons__,%d,%d)\n%%?LI%%LI%%xd(i,%%LI)\n /* the icon */ +%%Vl(a,%d,%d,%d,%d,1)\n%%?Lc%%Vs(gradient)%%s%%LT\n /* draw the text */, +screens[screen].getwidth(), font_height+padding*2, +padding, icon_size, icon_size,icon_size, icon_size, +icon_size+2, padding, screens[screen].getwidth() - icon_size+2 - icon_size, font_height); +printf(%s, buf); return buf; }
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 9 October 2011 21:03, Thomas Martitz ku...@rockbox.org wrote: Am 09.10.2011 09:38, schrieb Jonathan Gordon: 30min of fiddling around, attached is the screenshot from the e200 sim and the patch to do it, notice there is no code needed anywhere outside of the skin engine (infact its only done there because that pulls in the generated .sbs if none is specified). No actual code needs changing. Notes about the screenshot: * I'm not sure why the images arent being displayed, that might be a skin engine bug, more likely PEBCAK. * Gradient isnt completly setup in that example Thanks for this, this actually proves that the skin engine is largely unable to do it currently. Except for the points you mentioned, if you tried it on a touchscreen target (what this patch is all about) you would have realized that this is completely broken (no kinetic scrolling, no scrollbar, no list title, I even got it to crash). That the line selector draws over the line and not only the text is a significant portion of the patch which is needed in any event. FWIW, even if the skin engine would be able to do it. This patch isn't about theming. It makes Rockbox usable independantly of the current theme (though, it doesn't prevent themes from overriding it). Best regards. Did you completly ignore the part where I said it was a *quick proof* that your patch is wrong? I've already admitted that the skin engine is lacking some minor issues *which need to be fixed anyway*. Of course instead of fixing actual issues you want to do another half assed and wrong patch. :
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 9 October 2011 21:32, Thomas Martitz ku...@rockbox.org wrote: Am 09.10.2011 12:24, schrieb Jonathan Gordon: Did you completly ignore the part where I said it was a *quick proof* that your patch is wrong? I've already admitted that the skin engine is lacking some minor issues *which need to be fixed anyway*. Of course instead of fixing actual issues you want to do another half assed and wrong patch. : The issues I found are not minor. I can add to the list that the item you click is not what's being selected (it looks like the code that calculates the selection doesn't take the empty space into account), which is a pretty major issue. Why are you saying it's another half assed patch? Best regards. The issues you mentioned are things that need to be fixed on skin lists anyway (or that i didnt bother implementing in the 5 line snippet). Take your pick: Its a half assed patch because: a) Either its a usability feature - in which case completely ignoring the skin engine makes no sense as its hard to argue that the skin isnt the thing that the user interacts with 100% of the time. Or is a theme issue in which case doing it only for the inbuilt list is absolutely incorrect as it is sort of being phased out in favour of skinnable lists (or indeed native lists). b) It's a quick band aid fix for your single use case simply because noone has got round to fixing the issues which prevent this being done by the skin engine (which need to be done anyway), in effect adding nothing but bloat and more hacks into an already messy API (the lcd/viewport framework). I wouldnt be arguing so aggressively if I hadnt spend the better part of the last X years working *in the code which this deals with* if that doesn't count for anything then I don't see the point of the thread I have no issue with the actual feature, just the implementation (and even then, the configuration bit is fine). To do it correctly: * get kinetic scrolling working in skin lists (actually, move it up one layer so it works directly on the list widget and not the drawn list. I don't know how it works or what needs fixing to make it work (needs to be done anyway so is irrelevant to this topic) * get a lcd draw mode working so transparent doesnt use the background image (again, something which has been a wishlist item for ages, and mostly irrelevant to this topic) * add the logic to automatically create the sbs (finish my attached patch). * Add logic to extend custom skins to automatically deal with this (trivial to do, just resize the %Lb rectangle) Nothing I've said is hard or contentious, just a different way of dealing with the problem. Kindest regards
possible fix for skin buffer issues
Braindump for someone to pick up to fix the skin buffer memory allocation issue. The problem is that The vast majority of the skin buffers static allocation goes to the struct skin_element which is allocted for every single item in the skin file. Every line, tag, subline, comment, etc. on e200 this comes out to 41KB (going from memory). The next biggest allocation is struct skin_token, which is 20x smaller in total than skin_element. So obviously there is no point doing anything except move skin_element onto buflib. This is *problematic* because they are used, well everywhere. My braindump fix is this: 1) give the lib a callback which will allocated 1000 skin_element structs in a block (we obviously dont want 600 allocations for this) 2) in skin_alloc_element() allocate from the above array instead of the skin_buffer (when it is full allocate another 1000 or so) 3) in skin_render_viewport() (and a few other places) lock all those handles 4) in the move callback, walk the entire skin tree (this means every tree loaded) and update each pointer. This is going to be slow and crap, but hopefully not done often. I *think* those items are only used when traversing the tree so it is possible the pointer updating could even be done delayed (so store the diff somewhere and update the first time it is traversed after the move - Though this could be very dangerous, actually yeah, dont do this. hidden viewports are ignored at render time). I'm not sure what the alternatives are, this is a pretty nasty solution. Hopefully someone comes up with a better one (or implements this as I don't particularly have interest in doing it right now, but thought it would be good to explain the issue and a solution.) Jonathan
Re: possible fix for skin buffer issues
On 9 October 2011 23:48, Thomas Martitz ku...@rockbox.org wrote: Am 09.10.2011 14:37, schrieb Jonathan Gordon: 3) in skin_render_viewport() (and a few other places) lock all those handles Is locking even needed? The lcd functions (except the final lcd_update()/_rect()) don't yield(). Best regards. This is where we disagree Locking when not needed is far better than not locking when it is.
Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot
On 10 October 2011 01:38, Thomas Martitz ku...@rockbox.org wrote: Bah, Move the whole System menu into settings and call it whatever you want. It really doesnt deserve such a high placement in the menu system Is this a strong objection, or just stating that the patches don't go far enough in your opinion? Best regards. My position since the start of this thread has not changed. Any patch which doesnt remove either settings or system from the top level should not be done. I don't care what it gets called or how they are merged, but moving one item s a waste of time and jut means it is less likely they will get merged properly.
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 8 October 2011 05:18, Thomas Martitz ku...@rockbox.org wrote: Hello folks, I finally uploaded the patch that's lived long in my git repo (although in very hacked together fashion). I'm making this post here too since I want to get it in quickly. So please have a play with it and speak up if you have objections. This patch adds line padding to lists on touchscreens, in order to make lists reasonable useful without huge fonts. It's configurable: * Automatic (default, line height calculated using a lcd dpi aware function) * Off (status quo, line height = font height) * X pixels (from 2 to 50 in even steps) The automatic setting should/aims to Just Work Out Of The Box on all targets IMO this patch is needed to make Rockbox on touchscreen/RaaA remotely usable. Please comment or make suggestions! Best regards. This will probably be ignored but meh. This is the wrong patch for 2 reasons: 1) this can already be done in the themes 2) the only reason this is wanted at all is because Raaa doesnt use native lists. The effort shold be put into making native lists work (which is trivial once someone figures out how to draw the List in the Canvas rockbox uses. androids list api and rockbox's is pretty similiar (similar enough that a mostly thin wrapper is all that is needed).
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 8 October 2011 23:36, Dave Hooper d...@beermex.com wrote: If this is already possible in the theme, then I agree with Jon and don't see the need for the patch. Putting the padding into the theme itself makes more sense to me, since the theme author will already appreciate what spacing works/doesn't work, for that specific device, for that specific theme. I will admit that putting it into the theme makes it more annoying for users to modify, but then they will have to do it anyway for any theme which uses skined lists (as this patch would break all themes if it wasnt ignored by skins, which iiuc it is). However, that is a very fixable solution, either in code at parse (or even run)time, or with a script on the PC to modify a skin file (or a plugin on the DAP). Further, the internal sbs is already autogenerated from the user settings, so even if this (mis)feature was really wanted, all it would require is the setting and half a dozen lines in apps/gui/statusbar-skinned.c (in sb_create_from_settings() ) to setup the list viewport correctly. This would also then allow apps/gui/bitmap/list.c to be heavily butchered as *all* lists would be drawn by the skin which means much cleaner code and no more code duplication. Jonathan
Re: FS#12321 - Touchscreen: List line padding, to more easily select lines
On 9 October 2011 07:43, Torne Wuff torne+rockbox...@wolfpuppy.org.uk wrote: On 8 October 2011 11:47, Jonathan Gordon jdgo...@gmail.com wrote: On 8 October 2011 05:18, Thomas Martitz ku...@rockbox.org wrote: * Automatic (default, line height calculated using a lcd dpi aware function) 1) this can already be done in the themes The automatic setting *can't* currently be done in themes, since themes are only resolution dependent, not dpi dependent. Android devices have a pretty wide range there... -- Torne Wuff to...@wolfpuppy.org.uk Is that a challenge? :) everything about this patch can already be done in svn, and the configuration can be added. This is *not* a useability thing because it is only going to work reliably (err, at all) for themes which *dont* use skinned lists (which hopefully wont be very many once themers get the hang of them). On 9 October 2011 07:31, Frank Gevaerts fr...@gevaerts.be wrote: 1) While it's true that there has been work in that direction, I don't think it's entirely fair to say it can be done today. Skinnable viewports still have some issues, and I'm not convinced we have all infrastructure in place to fully implement the default lists. nothing impossible to fix. Anyway, dead horse beaten. This patch adds nothing which cant already be done.
Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot
On 9 October 2011 12:31, Thomas Martitz ku...@rockbox.org wrote: Am 23.08.2011 01:08, schrieb sideral: So here's the plan: * Move the entire Time Date menu out of System to Settings * Rename System to About * In the Time Date menu: * Sleep Timer offers the last-used timer value as its default. (This value is made persistent by way of the settings code.) When the timer is running, the entry changes to Cancel Sleep Timer (hh:mm), showing the remaining time. * Add Start sleep timer on boot option after Alarm Wake up Screen. I uploaded a patch series to the task doing exactly this. I intend to commit it within the next week (perhaps wednesday) someone speaks up against (please also do if you have already voiced your opionion). I just don't want this to die. Best regards. Bah, Move the whole System menu into settings and call it whatever you want. It really doesnt deserve such a high placement in the menu system
Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine
On 5 October 2011 18:19, Thomas Martitz ku...@rockbox.org wrote: Am 28.09.2011 09:00, schrieb Thomas Martitz: TBH, I would like to revert this commit. Best regards. Alright, as a few people agreed with me I'm going to revert this in a few hours. Best regards. Go ahead, just remember you're then responsible for breaking users ability to load their otherwise working themes. Frankly the 2 users whom I've personally helped trumps your (very likely personal) reasons against the change. Heck, read the thread, you're the only one who has said anything about the actual feature, everyone else just doesnt like the implementation (myself included to be completely honest). But unless you're going to do something about the apparent ram waste, or reimplement this is a nicer way, then you reverting is just a dick thing to do. Best regards to you too :) Jonathan
Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine
On 5 October 2011 18:50, Hayden Pearce saint.lascivi...@gmail.com wrote: I found the wording of a few people agree, so I'm reverting amusing. The amount of times a few people agreed something should be committed, and it wasn't, most often blocked by the same one or two people, vs the ease of the descision to revert something here is quite funny. sits back I think the word you want is irony :)
Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine
I already explained why it was needed, noone replied to that email so apparently noone actually cares. I see its been reverted, oh well. On 6 October 2011 01:23, pondlife pondl...@pondlife.me wrote: is just a dick thing to do Hrmf, don't be a shitweasel.* Reverting this is definitely not personal, I thought you agreed that this was a badly implemented semi-secret feature too. I'm sure those 2 users can use 3.9.1 in the meantime, while we try to get to a fully stable 3.10 release. pondlife p.s. I apologise, sincerely**. (*Ironic use of entirely inappropriate personal abuse, for which I should apologise.) (**Non-ironic; I really don't like this personal abuse thing - I'm sure there are dedicated MLs elsewhere for that sort of thing.)
Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine
On 6 October 2011 14:10, Dave Hooper d...@beermex.com wrote: I did reply to your email. I could repost my response if you think it would be useful. You replied to the thread, but not to the email explaining the situation. Someone may as well go and bump the #define in skin_engine.h now as I'm pretty confident noone is going to bother working on a better solution.
Re: Im out
On 28 September 2011 21:54, XavierGr xavie...@gmail.com wrote: On 28 September 2011 10:15, Jonathan Gordon jdgo...@gmail.com wrote: I'm saying this publicly because I don't want anyone to think I'd simply lost interest in the project or found another itch elsewhere. I very much have stuff I want to keep working on and am leaving because of (mostly) 2 individuals. I'll continue working on rockbox on my git repo, and of course you're free to cherry-pick the changes you want, but I wont be pushing changes back anymore (not regularly anyway). If you are going to continue working on Rockbox on a personal repository, why do you have to leave? Your contribution to this project has been huge all these years, why deny future features from the userbase for just 2 controversial people. It is certainly frustrating to have to develop under the pressure of such thankless and sometimes aggressive behaviour, but we are talking about a very small (albeit vocal) minority. In the end, it mostly sounds like you want to avoid further controversy and not that you just lost interest to the project, which for me seems rather unfortunate. If I continue fiddling with rockbox it will be outside of the main repo simply because it means I can do the changes I want without being accused of pushing horrible aggressive agendas. It feels like every discussion I've been involved in the last 6+ months has simply turned to poo-flinging with the same few individuals. I'm not saying I didnt throw my share too but untill recently I did my best to keep emotion out of it. After however many months of trying to keep things civil I just don't see the point anymore. Probably the biggest frustration was the majority of devs who simply said nothing. Obviously the constant arguments reflect badly on the community but noone seemed to want to throw in their weight and force an outcome (whether it be drop it or let him do the change). The RSB is supposed to be a post-crisis response but it has failed in that by the time it is called it is already too late. My second biggest annoyance is the loud minority of non-contributing devs who stand in the way of progress. (of course non-contributing dev is a bit of an oxymoron, but people who did some work months/years ago and do little more than chat in irc really should learn to shut up and get out of the way of the active developer group). Finally, the general aggression amongst some of the regular devs. This includes a disturbingly high level of passive aggression in irc. Until people can be trusted to keep an aggressive tone out of simple discussions there is simply no point talking to them. (case in point is kugels reply to the playback gsoc thread, or the amazing response I got to the *suggestion of talking about* of code reviews in gerrit). I know for sure some people are reading this thinking I'm begin a massive hypocrite, but then those people are the very reason I've left. So, To the people nice enough to say thanks and try to try my mind, fix the above and we'll see (I'd be incredibly surprised if anything changes). I do apologise for the timing a bit, I know the font commits brought some instability which I feel a bit bad about (Another issue with the group is the tiny proportion of devs willing to test other devs' patches), but it looks like there is always someone else ready to pick up the ball. I'll continue to be in IRC and I'll happily reply to rockbox related emails for the code that I wrote/maintained. Jonathan
Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine
On 28 September 2011 17:24, pondlife pondl...@pondlife.me wrote: TBH, I would like to revert this commit. FWIW, I agree. A solution is needed to this memory waste, but this isn't it. pondlife Very quickly, the skin buffer always was a memory hog, but now that the big items (images, fonts) are in buflib there is completly no sensible size that can be used. something like 50% of the usage is now on the tree elements which are alloced once for *each* item, ilike on the nano does 41KB for that while cabbie does 15ish. there is no way to reduce this by enough to make it worth it (reducing the struct by 4 bytes still isnt enough to make a real dent). if someone were to make the whole skin system buflib aware then ok, but that isnt going to happen, this solution (admittedly quirky) is the only solution that allows people to enjoy *big* themes without forcing people who want very bare theme to lose a considerable amount of buffer space. So go ahead and revert it if you all care that much. I ignored the thread initially because of the attitude and not the suggestion. Had it been brought up in a civil manner then things may be different (Though unlikely). Hope someone picks up skin_engine maintenance soon.
Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine
I'm happy to talk about technical issues... On 26 September 2011 16:15, Amaury Pouly amaury.po...@gmail.com wrote: Hi All, Furthermore this user setting is implemented in a totally obscure way (a magic file containing a magic number, which isn't checked if it's even plausible). It's pattern that doesn't exist yet, while there's a clear existing implementation of user settings. It's rather hidden without a chance to be discovered by the user. I don't think I need to mention that the manual doesn't tell anything about this. I don't like the way this setting is implemented, we have a framework for that and see no good reason to not use ot. Regards It doesnt really sit well with the current setting system, and I need to double check settings are loaded early enough (which is mostly the reason I did it this way). I'll have another look though as yes if it can use config.cfg it would be better.
Re: The next release version
a) 3.10 b) because history says that whatever I suggest will be outright rejected On 25 September 2011 09:59, Alex Parker parker.ale...@gmail.com wrote: Hi guys, So the next release is due towards the end of October, and in the past there has been some discussion as to whether the version number should be 3.10 or 4.0. Myself I lean towards 3.10, and keeping 4.0 for e.g. an Android release but I am not too determined either way. What do people think? I'd appreciate a two part answer: a) 3.10 or 4.0 b) why? Cheers chaps, looking forward to your input :) Alex
Re: The next release version
serious response: a) 4.0 b) 1 - there has been plenty of stuff commited in the last release which gives us a good enough reason to bump the major number 2 - 3.10 looks a bit funny 3 - no other target has caused a major bump so why should android be different? 4 - the numbers are pretty arbitrary anyway On 25 September 2011 09:59, Alex Parker parker.ale...@gmail.com wrote: Hi guys, So the next release is due towards the end of October, and in the past there has been some discussion as to whether the version number should be 3.10 or 4.0. Myself I lean towards 3.10, and keeping 4.0 for e.g. an Android release but I am not too determined either way. What do people think? I'd appreciate a two part answer: a) 3.10 or 4.0 b) why? Cheers chaps, looking forward to your input :) Alex
Re: The next release version
2011/9/25 Jonas Häggqvist ras...@rasher.dk: +1. 2.x-3.x was when SWCODEC was added (and initially 3.0 was planned for Archos+Iriver Hxx0 only IIRC). I'd say the addition of (releasable) Raaa is on the same level. From a technical point of view, RaaA is a pretty uninteresting target. SWCODEC brought an entirely new playback engine, RaaA brings very little. On the other hand the user-hidden changes which have gone in recently easily could qualify for a major bump (of course the number is entirely artificial anyway so whatever). Things like the software mixer which *finally* fixed the very long standing bug of no voice while playback was paused, buflib in core, skinlists (so the entire ui is themeable), etc
FS#12273 - load fonts into buflib needs testing!
Hi all, http://www.rockbox.org/tracker/task/12273 Is very ready for mass testing so please everyone get it into your private builds and help out! What this does is use buflib instead of a static array for *all* fonts loaded by the system. This is a big patch and im not really willing to just commit and fix bugs later. All you need to do to test is put it in your build and use your DAP as usual. changing themes and fonts would be useful too! :) I'd like to commit this before the end of the week. There is one known issue where changing themes could make one line in the theme chooser list dissapear for some unknown reason! leaving the menu clears it up, and as I have no idea where its coming from, and it isnt a crash bug I'm not too concerned, though if anyone can reliably crash it I'd be very intersted in repro steps. Cheers Jonathan P.S please remember to get the latest version of the patch :)
Re: Git/gerrit migration status and next steps
On 7 September 2011 00:40, Torne Wuff torne+rockbox...@wolfpuppy.org.uk wrote: 4) We write up policies and documentation on how to use git via gerrit, though only two parts are crucial at this point: how to clone the repositories, and how committers can commit directly to master. Do we really want to allow commits directly? so little of our code ever gets reviewed so I would quite happily force everyone to go through gerrit and require someone else to OK it. It isnt hard to get someone else in IRC to have a quick look and push the button.
RE: Git/gerrit migration status and next steps
On Sep 7, 2011 5:43 PM, bryan.chi...@rbs.com wrote: Do we really want to allow commits directly? so little of our code ever gets reviewed so I would quite happily force everyone to go through gerrit and require someone else to OK it. It isnt hard to get someone else in IRC to have a quick look and push the button. Absolutely against requiring this. We trust committers to do the right thing. If you have a patch you think needs this - submit to refs/for/master. For small commits, this would be a completely pain in the arse, and totally unwarranted. Bryan Childs This isn't about people doing the wrong thing. This is about trying to get code quality. And yes it will be a minor pain sometimes, but a small fix has just as much fuck-up-ability as a big one and is *easier* to sanity check then the required debugging later. This is something gerrit makes easy and we should absolutly do it. Remember that you push directly to gerrit, this doesn't require you to open your browser.
Re: Git/gerrit migration status and next steps
On 7 September 2011 21:02, Nils Wallménius nils.wallmen...@gmail.com wrote: On Wed, Sep 7, 2011 at 10:53 AM, Thomas Martitz ku...@rockbox.org wrote: Am 07.09.2011 10:29, schrieb Nils Wallménius: I think it could be interesting to test it actually. Testing it will probably show that it works in the beginning, but it will not show that this way degrades to clicking away commits like ads (without even looking) in the long (or even short) term. We can't force people to actually review. And we can't force them to do it properly. This way has no hope of working as intended. What it may work for is to open the ability to point at someone else for every controversial/bad/unwanted commit. This even reduces the motivation to review (and to work on Rockbox in general). Best regards. That's the whole point of a test, to see how it works out, you seem certain that it *can't* work which i find overly negative. Of course we can't force people to do anything but i think this will encourage people to review, if it turns out it doesn't work, we disable it. I don't see why people have focused on the blame-sharing as someone called it in IRC how many commits do we get that are actually bad? Anyway, i agree with Torne about taking one step at a time. Nils I honestly have no idea how the whole blame shareing thing happened either. anyone who does software engineering should know that review means implementation review and not feautre review. I also mentioned it now because it is a valuable thing to talk about, I gess I should use an alias now seen as obviously everything I suggest is so unbeleivably wrong!
Re: Git/gerrit migration status and next steps
On 7 September 2011 21:38, Nils Wallménius nils.wallmen...@gmail.com wrote: I guess we will see if people use the review stuff voluontarily but i expect not as it's easier to just push directly (as we do it now). If everyone has to get their changes reviewed i think that would make people more inclined to review other people's chnges. But that's just a guess and as i said i would find it interesting. Nils My intuition says that commiters will use it exactly like flyspray, i.e very rarely. I am looking forward to being able to push new contirbuters patches more quickly though.
Re: Settings reordering.
On 25 August 2011 22:50, Paul Louden paulthen...@gmail.com wrote: Okay, what I'd like to see here is this: People who actually want to directly contribute to this process (filling out a list of categories at each step, etc), please respond to this saying so, so that we can come up with dates and get this thing moving, as well as trying to begin selecting our conflict resolution group (which we won't need immediately, but will need at the end of the first phase). Well, its been 9 days with nothing happening. This is pretty much dead in the water unless people who have said they want to help start doing something.
Re: Discussion regarding reordering the main/root menu
On 29 August 2011 16:22, Johannes Linke johannes.li...@gmx.de wrote: As users will need Settings way more often than System, it seems absurd to give System a higher priority and move Settings one level down. From my origional email: As a compromise I would accept removing the settings submenu and moving all those items up one level if that was what was more preferred. -- I also dont necessarily agree with your statement
Re: Discussion regarding reordering the main/root menu
On 29 August 2011 03:13, Al Le al...@gmx.de wrote: That said, a patch to even just allow simple reordering of the main menu isnt going to be trivial so we should figure out a better layout I once tried to do exactly that (in FS#6718 (only the main menu) and FS#7809 (general menu reordering with the option of hiding)), but both patches were rejected. I don't remebmer why exactly -- because it is (or was) a forbidden feature or because of the way it was implemented. Just looking at FS#6718 now (it probably still applies cleanly!). It's not the way I would have done it, but I see no reason other than our customisation-phobia to not include it. a plugin frontend could easily be added to make it more user friendly also. I'm a bit scared to look at the other one though. I read automatic reordering and get shivers :) Anyway, I'd like to try and talk about the static order first, then (or at least separately) talk about giving the user the choice. I know this thread hasnt been going long but it would be nice to hear from more people. we have half a dozen very similar proposals and conservative guesses suggest 40+ active developers of which most havnt said a word one way or another.
Discussion regarding reordering the main/root menu
*Everyone* agrees Time Date does not belong in system as it currently is. Hypothetically, if it were moved out of System and put into Settings it begs the question as to the point of the system menu at all. Most people agree Rockbox Info is the most important item in that menu, and running time can be removed (I've heard noone spak out against that suggestion). The remaining two items are debug (keep out) and credits. My view regarding debug is that there is nothing there that is actually useful once a port is up and running *or during development* when dealing with custom builds anyway so disabling that by default (but adding an easy configure item to enable it) removes clutter. Credits is only there because it should go somewhere and nowhere else makes any more sense. So hypothetically system is down to just rockbox info and credits. Why are they given such a high priority by being directly accessable from the main menu? Equally, why are the settings given such high priority in the main menu? Sure they are useful, but after the initial player setup how many settings does one need access to? and given how poor the setting layout is it is likely regularly used configs are saved in .cfg anyway (and put on the quickscreen). My suggestion has always been that System and Settings be merged in some meaningful way. My favored layout is then: System: Rockbox info Time Date Settings Sound settings playback settings general settings recording settings theme settings manage settings Credits Debug As a compromise I would accept removing the settings submenu and moving all those items up one level if that was what was more preferred. Now, while we are here it makes sense to talk about the order of the other items deamed to be the most important parts of the UI. I think it makes sense to group items based on what they give the user. Files and Database are Find music to play and should be grouped. WPS, radio, recording are listen to music and should be grouped. System and Plugins are misc and should be grouped. Playlist catalog and bookmarks are find previously saved playlists and should be grouped. The order I'd like to see is then: Files Database WPS Recording FM System Plugins Playlist Catalogue Bookmarks Just about every player has direct access to the wps/fm screen with a single button so they are sensible to put in the middle. Files/Database are the most often accessed (based on a number i pulled from my tuches), system is the least important and so in the middle below music, PC and bookmarks are relativly important and thanks to the list wrapping very accessible. An alternate main menu which I would like moves P.C and bookmarks between database and WPS so all the ways to find music are grouped together (though thinking about it the first one makes more logical sense to me). There are two final options. Do nothing. And move TD into settings and call it done. My views of both are well know hence the above proposals. Discuss
Re: Discussion regarding reordering the main/root menu
On 28 August 2011 23:22, Paul Louden paulthen...@gmail.com wrote: Equally, why are the settings given such high priority in the main menu? Sure they are useful, but after the initial player setup how many settings does one need access to? and given how poor the setting layout is it is likely regularly used configs are saved in .cfg anyway (and put on the quickscreen). I don't really think your speculation on what the most likely way users use the settings is means much. The honest truth is, we don't know. People have asked for a quickscreen that can hold more settings, meaning that there's at least 5 settings they want to change regularly. There are many people who didn't even know you could save custom .cfg files. We have no real way of knowing how the typical user experiences Rockbox. Even a poll would give a decidedly skewed result. We should be making our decisions based on something else. Perhaps what will help a new user most easily find what they need. I'm not saying that explicitly, but rather, forward-looking ideas rather than I think this is how users already use our software guesses. That is all a failure of the manual then, that's not to say the menus aren't to blame also (we agree the settings menus suck), but not understanding config files is a pretty fundamental failure of the manual. I think it makes sense to group items based on what they give the user. Files and Database are Find music to play and should be grouped. WPS, radio, recording are listen to music and should be grouped. System and Plugins are misc and should be grouped. Playlist catalog and bookmarks are find previously saved playlists and should be grouped. I don't see how grouping is beneficial. Is the idea that they need to be in groups to make it easier for users to spot them? Are users even going to recognize that playlist catalog and bookmarks are the same group? I certainly don't see bookmarks as a playlisting function, at least. Recording isn't related to listening to music at all, and is arguably the exact opposite. The groups seem a little arbitrary in the first place, but I don't see a reason grouping is the approach we should take with this list. Well of course the list is entirely arbitrary. My suggestion was to try to add some logic to it. I can't be the only one to have an opinion on this, so why dont people respond with their ideas? To me, rather, two things should be taken into account. What's visible on the screen on our smallest screen targets with the default theme, and what individual items are most likely to be sought. The screen visibility seems important because when a user first turns on our player, especially during the period they're unfamiliar, they're going to want to glance at the screen and see useful options. Not remember whether they need to go up or down to get to the ones they want. I think it's convenient for the most used options to be the least presses away, but visibility on boot is a little more important. Indeed that should be a consideration but not *the* major one. That being said, the below list isn't bad. On the Clip+ though, the menu is six items I believe. This means Recording, FM and System are visible on boot where Bookmarks and Playlist Catalogue are not. I'd actually recommend moving FM below WPS, and squeezing those two between FM and Recording. I don't mean to belittle the value of recording, as such, but many of these devices don't even have the feature, and even those that do are mostly sold as music players. Features like Bookmarks and Playlists should be made more visible/accessible. Someone who's bought a unit primarily for recording purposes probably checked that Rockbox can still record before installing it, and will scroll down to look for it.. Bookmarks is hidden by default. Now the real question is, why are we so bone-headedly against customisable (main)menu layouts? I personally never use the database, recording or fm so why must they have priority in my list? I know for sure others use the database primarily and see the files as being arrogantly in the wrong place. Additionally, why are we so against changing the quickscreen to be a list for those that want it like that? That said, a patch to even just allow simple reordering of the main menu isnt going to be trivial so we should figure out a better layout, or admit we have no clue and leave it as it - which still leaves us with the question of system/settings merging. The order I'd like to see is then: Files Database WPS Recording FM System Plugins Playlist Catalogue Bookmarks
Re: Discussion regarding reordering the main/root menu
On 29 August 2011 02:04, Johannes Linke johannes.li...@gmx.de wrote: Hi, why can't you just move System into Settings? Ok, credits are not a setting, as well as Rockbox info isn't. But look at this from a users point of view: meaningful menu item names are for remembering where to look for something. If you're searching for settings, you will find them quickly because the menu item is called Settings. But who searches for Credits? Nobody. And who searches for Rockbox info? The only info which is relevant to a user is battery level and amount of space used on disk. The battery level is shown in the statusbar and in most WPS's, so I woudln't worry about that. And since most people do massive data moving with their PC, i wouldn't consider disk space used to be problematic. My proposal: Move System into Settings. The name inconsistency is negligible, as names are for remembering where to find certain things, but nobody needs to find System. I'm quite certain that not a single user will complain about that move. The only problem I see is, that some developers can't stand inconsistencies. Regards, Johannes I find it very hard to imagine a user who was cluey enough to find rockbox but then not be able to find the settings under the system menu. Gnome2 puts everything under System, Windows put its under Control Panel. Bad names are always bad when there are alternatives.
Re: SUMMARY 2: FS#10849 - Sleep timer options: persistent duration and start on boot
On 28 August 2011 06:15, Thomas Martitz ku...@rockbox.org wrote: Its unfortunate that you let the single hypocrite win. Really everyone else agreed with moving td to settings. Perhaps I'll just move it afterwards...just cause. Best regards. how to win friends and influence people (y)
Re: Remove the System menu!
On 23 August 2011 18:29, Jonathan Gordon jdgo...@gmail.com wrote: Hi, This is my view on the other thread about moving the Time Date menu. Get rid of the System menu. it is entirely pointless. The items hat will be in it once TD is moved (which sounds like a given) are: Rockbox info, credits, running time, debug. Rockbox info has very little actually useful information, credits can be got from the plugins menu, running time is nothing more than a waste of binary space, and debug serves basically no purpose and should be disabled by default (We've already talked about disabling it in release builds). What I'd much prefer to see is the current settings menu be renamed to system and either: 1) settings be a submenu, or 2) just copy all the items from the current system menu into the new settings/system menu. The main menu order should then be: * (hidden bookmarks) * Files * Database * playlist catalogue * resume playback * plugins * system and now the main menu makes some sense. Items are prioritized from the top down starting with ways to get to your music on to less important items. Resume playback i pushed down because just about all targets have a dedicated button to get there. I'd be happy to swap plugins and system. If the system menu didnt have settings as a submenu it would look like this (making it very obvious what is and isnt a setting): * rockbox info * sound settings * playback settings * general settings * Time Date * theme settings * manage settings * running time (I'd rather nuke this though) * debug menu (remove also) * Credits Jonathan As usual, the only way to get any discussion is to commit and fend off the hordes in IRC after. So, This patch will be commited tonight (if I have time) or sometime over the weekend. All this does is move the contents of the settings menu into the system menu, and move the system menu to where settings currently is. It also changed the rockbox info icon. No other changes. With this done the sleep timer patch can put all its settings in the TD screen with no issues. Jonathan diff --git a/apps/menus/main_menu.c b/apps/menus/main_menu.c index c5758d1..91d6411 100644 --- a/apps/menus/main_menu.c +++ b/apps/menus/main_menu.c @@ -108,6 +108,32 @@ MAKE_MENU(manage_settings, ID2P(LANG_MANAGE_MENU), NULL, Icon_Config, /**/ /***/ +/*MAIN MENU*/ + + +#ifdef HAVE_LCD_CHARCELLS +static int mainmenu_callback(int action,const struct menu_item_ex *this_item) +{ +(void)this_item; +switch (action) +{ +case ACTION_ENTER_MENUITEM: +status_set_param(true); +break; +case ACTION_EXIT_MENUITEM: +status_set_param(false); +break; +} +return action; +} +#else +#define mainmenu_callback NULL +#endif +/*MAIN MENU*/ +/***/ + + +/***/ /* INFO MENU */ @@ -390,7 +416,7 @@ static bool show_info(void) return simplelist_show_list(info); } MENUITEM_FUNCTION(show_info_item, 0, ID2P(LANG_ROCKBOX_INFO), - (menu_function)show_info, NULL, NULL, Icon_NOICON); + (menu_function)show_info, NULL, NULL, Icon_Rockbox); /* sleep Menu */ @@ -440,50 +466,22 @@ MENUITEM_FUNCTION(show_runtime_item, 0, ID2P(LANG_RUNNING_TIME), MENUITEM_FUNCTION(debug_menu_item, 0, ID2P(LANG_DEBUG), (menu_function)debug_menu, NULL, NULL, Icon_NOICON); -MAKE_MENU(info_menu, ID2P(LANG_SYSTEM), 0, Icon_System_menu, +MAKE_MENU(info_menu, ID2P(LANG_SYSTEM), mainmenu_callback, Icon_System_menu, +show_info_item, +sound_settings, +playback_settings, +settings_menu_item, theme_menu, +#ifdef HAVE_RECORDING +recording_settings, +#endif +manage_settings, #if CONFIG_RTC timedate_item, #endif - show_info_item, show_credits_item, show_runtime_item, #if CONFIG_RTC == 0 sleep_timer_call, #endif + show_credits_item, show_runtime_item, debug_menu_item); /* INFO MENU */ /***/ - -/***/ -/*MAIN MENU*/ - - -#ifdef HAVE_LCD_CHARCELLS -static int mainmenu_callback(int action,const struct menu_item_ex *this_item) -{ -(void)this_item; -switch (action) -{ -case ACTION_ENTER_MENUITEM: -status_set_param(true); -break; -case ACTION_EXIT_MENUITEM: -status_set_param(false); -break; -} -return action; -} -#else -#define mainmenu_callback NULL -#endif -MAKE_MENU(main_menu_, ID2P(LANG_SETTINGS), mainmenu_callback, -Icon_Submenu_Entered, -sound_settings, -playback_settings, -settings_menu_item, theme_menu, -#ifdef HAVE_RECORDING
Remove the System menu!
Hi, This is my view on the other thread about moving the Time Date menu. Get rid of the System menu. it is entirely pointless. The items hat will be in it once TD is moved (which sounds like a given) are: Rockbox info, credits, running time, debug. Rockbox info has very little actually useful information, credits can be got from the plugins menu, running time is nothing more than a waste of binary space, and debug serves basically no purpose and should be disabled by default (We've already talked about disabling it in release builds). What I'd much prefer to see is the current settings menu be renamed to system and either: 1) settings be a submenu, or 2) just copy all the items from the current system menu into the new settings/system menu. The main menu order should then be: * (hidden bookmarks) * Files * Database * playlist catalogue * resume playback * plugins * system and now the main menu makes some sense. Items are prioritized from the top down starting with ways to get to your music on to less important items. Resume playback i pushed down because just about all targets have a dedicated button to get there. I'd be happy to swap plugins and system. If the system menu didnt have settings as a submenu it would look like this (making it very obvious what is and isnt a setting): * rockbox info * sound settings * playback settings * general settings * Time Date * theme settings * manage settings * running time (I'd rather nuke this though) * debug menu (remove also) * Credits Jonathan
Re: Remove the System menu!
On 23 August 2011 18:29, Jonathan Gordon jdgo...@gmail.com wrote: Hi, This is my view on the other thread about moving the Time Date menu. Get rid of the System menu. it is entirely pointless. The items hat will be in it once TD is moved (which sounds like a given) are: Rockbox info, credits, running time, debug. Rockbox info has very little actually useful information, credits can be got from the plugins menu, running time is nothing more than a waste of binary space, and debug serves basically no purpose and should be disabled by default (We've already talked about disabling it in release builds). What I'd much prefer to see is the current settings menu be renamed to system and either: 1) settings be a submenu, or 2) just copy all the items from the current system menu into the new settings/system menu. The main menu order should then be: * (hidden bookmarks) * Files * Database * playlist catalogue * resume playback * plugins * system and now the main menu makes some sense. Items are prioritized from the top down starting with ways to get to your music on to less important items. Resume playback i pushed down because just about all targets have a dedicated button to get there. I'd be happy to swap plugins and system. If the system menu didnt have settings as a submenu it would look like this (making it very obvious what is and isnt a setting): * rockbox info * sound settings * playback settings * general settings * Time Date * theme settings * manage settings * running time (I'd rather nuke this though) * debug menu (remove also) * Credits Jonathan The general consensus in IRC seem to be merging the two menus is a good idea and we are tied up on how and what to call it. Attached are 3 diffs: 1) move_settings_into_system.diff - does what it says, makes the settings menu a submenu of system 2) merge_menus.diff - merges the items of the two menus with a bit of order tweaking and calls it system in the main menu 3) merge_and_call_settings.diff - same as 2 except the menu name is Settings. This is to keep people happy although I think this is absolutly wrong. diff --git a/apps/menus/main_menu.c b/apps/menus/main_menu.c index c5758d1..467f477 100644 --- a/apps/menus/main_menu.c +++ b/apps/menus/main_menu.c @@ -108,6 +108,32 @@ MAKE_MENU(manage_settings, ID2P(LANG_MANAGE_MENU), NULL, Icon_Config, /**/ /***/ +/*MAIN MENU*/ + + +#ifdef HAVE_LCD_CHARCELLS +static int mainmenu_callback(int action,const struct menu_item_ex *this_item) +{ +(void)this_item; +switch (action) +{ +case ACTION_ENTER_MENUITEM: +status_set_param(true); +break; +case ACTION_EXIT_MENUITEM: +status_set_param(false); +break; +} +return action; +} +#else +#define mainmenu_callback NULL +#endif +/*MAIN MENU*/ +/***/ + + +/***/ /* INFO MENU */ @@ -390,7 +416,7 @@ static bool show_info(void) return simplelist_show_list(info); } MENUITEM_FUNCTION(show_info_item, 0, ID2P(LANG_ROCKBOX_INFO), - (menu_function)show_info, NULL, NULL, Icon_NOICON); + (menu_function)show_info, NULL, NULL, Icon_Rockbox); /* sleep Menu */ @@ -440,50 +466,22 @@ MENUITEM_FUNCTION(show_runtime_item, 0, ID2P(LANG_RUNNING_TIME), MENUITEM_FUNCTION(debug_menu_item, 0, ID2P(LANG_DEBUG), (menu_function)debug_menu, NULL, NULL, Icon_NOICON); -MAKE_MENU(info_menu, ID2P(LANG_SYSTEM), 0, Icon_System_menu, +MAKE_MENU(info_menu, ID2P(LANG_SETTINGS), mainmenu_callback, Icon_System_menu, +show_info_item, +sound_settings, +playback_settings, +settings_menu_item, theme_menu, +#ifdef HAVE_RECORDING +recording_settings, +#endif +manage_settings, #if CONFIG_RTC timedate_item, #endif - show_info_item, show_credits_item, show_runtime_item, #if CONFIG_RTC == 0 sleep_timer_call, #endif + show_credits_item, show_runtime_item, debug_menu_item); /* INFO MENU */ /***/ - -/***/ -/*MAIN MENU*/ - - -#ifdef HAVE_LCD_CHARCELLS -static int mainmenu_callback(int action,const struct menu_item_ex *this_item) -{ -(void)this_item; -switch (action) -{ -case ACTION_ENTER_MENUITEM: -status_set_param(true); -break; -case ACTION_EXIT_MENUITEM: -status_set_param(false); -break; -} -return action; -} -#else -#define mainmenu_callback NULL -#endif -MAKE_MENU(main_menu_, ID2P(LANG_SETTINGS), mainmenu_callback, -Icon_Submenu_Entered, -sound_settings, -playback_settings, -settings_menu_item
Re: Settings reordering.
I've already put my hand up to help out. Ideally as many comitters as possible say they are either in principle backing the committe and waiting for the results, against the whole notion of this change, or abstain completly (including the final vote). This is going to be alot of work for those who get involved and knowing what sort of opposition there is going to be from the start is important. It would also be good to get people who want to be activly invovled to put your hands up soon (i.e before the weekend) because we'd like to have this done for the next release feature freeze which is currently set for oct 17. On 23 August 2011 23:45, Paul Louden paulthen...@gmail.com wrote: The overall deadline for the project would be the beginning of the feature freeze for the next release, so that the new menu layout can be available with the next Rockbox release. Again, this whole process is up for comments before we start. If we get enough people willing to commit to contributing their suggestions at each phase, we'll take it and go. If you object to the results of the phase after the phase is over, don't waste time starting arguments, just vote against it at the end. If the version contributed to by everyone is voted down, then we can entertain voting on contributed .diffs, but the real goal here is to use a process by which we incorporate a little bit of everyone's input into something that is, if never near perfect, better than what we have now. Just on that last paragraph. Te vote should be a simple yes/no, with no veto-ing possible (unlike RFC-type posts which are expected for normal features). This is going to be alot of work., and we want as many people to be * actively involved* as possible, but we dont want people who dont have an open mind, or only want to do little changes. So, If you want in let us know. If you feel there is nothing wrong with the menus then that is fine, but please stay out of our way.
Re: Settings reordering.
On 24 August 2011 00:05, bryan.chi...@rbs.com wrote: I think it's worth trying to set a guideline for the maximum desirable menu depth too - I don't think we'd want to go to fine grained for everything unless it's just otherwise unworkable? Yes, but lets agree that we are going to do this first, then that and other questions need to be answered. Max list length is another important one
Re: Remove the System menu!
On 23 August 2011 22:39, Paul Louden paulthen...@gmail.com wrote: You should probably also mention that included in those diffs is also a rearranging of the main menu as you mentioned in your first message, but left out in your summarized descriptions here. Like you said, it was mentioned in the first post, to which noone has replied. I see this patch as orthogonal to the settings work and would like to get a answer on it to help the sleep time patch go through smoothly also. If that means taking out the playlist catalogue swap then so be it.
Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot
On 23 August 2011 09:08, sideral side...@rockbox.org wrote: Given that it looks like there's overwhelming support for (and little to no concern about) moving Time Date to Settings, I now think it's fine to do that change along with the proposed sleep-timer extensions. Also, I'd like to pick up Thomas Martitz's proposal and get rid of an explicit setting for the default sleep time in favor of the sleep-timer menu offering the last selected value as its default value. So here's the plan: * Move the entire Time Date menu out of System to Settings * Rename System to About * In the Time Date menu: * Sleep Timer offers the last-used timer value as its default. (This value is made persistent by way of the settings code.) When the timer is running, the entry changes to Cancel Sleep Timer (hh:mm), showing the remaining time. * Add Start sleep timer on boot option after Alarm Wake up Screen. sideral I disagree with the first part of this change. Time date doesnt make any more sense in settings than it does in system (or info if you want to rename it). timedate isnt a setting. And it does make sense to keep anything related (sleep timers and alarms) together,
Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot
On 23 August 2011 10:27, Paul Louden paulthen...@gmail.com wrote: On Mon, Aug 22, 2011 at 7:10 PM, Jonathan Gordon jdgo...@gmail.com wrote: I disagree with the first part of this change. Time date doesnt make any more sense in settings than it does in system (or info if you want to rename it). timedate isnt a setting. And it does make sense to keep anything related (sleep timers and alarms) together, Unless Time and Date became immutable while I wasn't looking and can't be set by the user, I'd say they're settings. It could possibly be argued that the screen where you view the time and date could be separated from the setting that allows you to set them, but there is absolutely a setting involved. The design of the setting menu may be non-standard in that it, for some reason, previews the values by showing them on layer above where you set them, but that doesn't instantly make it not a setting. What has how you set it got anything to do with it being a setting or not? Time does not magically change when you give your DAP to your sibling.