This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#5744 - Button press for USB connection or USB power only
Attached to Project:
Rockbox
Opened by Matthias Mohr (aka Massa) (mmohr) - Sunday, 30 July 2006, 14:57 GMT+2
Last edited by Frank Gevaerts (fg) - Friday, 12 December 2008, 21:59 GMT+2
Opened by Matthias Mohr (aka Massa) (mmohr) - Sunday, 30 July 2006, 14:57 GMT+2
Last edited by Frank Gevaerts (fg) - Friday, 12 December 2008, 21:59 GMT+2
|
DetailsHere is a patch which add another option to the "System/Battery" settings menu.
You can change the behaviour of the internally called "USB powerbutton": For iRiver H300 it is the REC button, For Ondios it is the MENU button, For iPods it is the MENU button, For recorders it is the F1 button. Normal behaviour is that you press the button when plugging in the USB cable to prevent it from USB connection and only use the USB power to charge your device. With this patch you can decide if it still works this way or if it works vice versa: if you then press that button it will initiate the USB connection (and if you don't press it, it'll just use USB power). Additionally it adds this new behaviour to the car mode. So if you switch on the car mode, you always have to press the button if you want to do an USB connection. The patch should work for all devices which can use the USB_POWER feature... Enjoy and tell me if it doesn't work! 
buttonpress_for_usb_connect_20060730.patch
(10.4 KiB)
apps/debug_menu.c | 7 +++++- apps/lang/deutsch.lang | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ apps/lang/english.lang | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ apps/misc.c | 9 ++++++++ apps/settings.c | 8 +++++++ apps/settings.h | 4 +++ apps/settings_menu.c | 39 ++++++++++++++++++++++++++++++++++++-- firmware/export/usb.h | 1 firmware/usb.c | 17 +++++++++++++++- 9 files changed, 181 insertions(+), 4 deletions(-) |
This task depends upon
Closed by Frank Gevaerts (fg)
Friday, 12 December 2008, 21:59 GMT+2
Reason for closing: Rejected
Additional comments about closing: Configurable buttons are a specific NODO, and simply reversing the behaviour would be _very_ surprising for most users
Friday, 12 December 2008, 21:59 GMT+2
Reason for closing: Rejected
Additional comments about closing: Configurable buttons are a specific NODO, and simply reversing the behaviour would be _very_ surprising for most users
I included the buttonpress patch (the merged one from the usb charging task) and experience the following strange thing: However I set the option, I always have to press the REC button for a connection. AND: USB charging is always done, even if the setting is turned off.
Can you take a look at this and tell me what you think? For my build including the patches I used see http://www.misticriver.net/showthread.php?t=42675
Thanks and all the best
Norbert
you're currently the only one where the patch doesn't work.
So I assume it interferes with some of your other patches!
I'm sorry that I don't have the time to have a deeper look
at your special build.
I suggest that you create a version with only my patch(es)
and tell me if it still does not work.
If it works then, you'll have to find out which other patch
make it unworkable. Maybe you try another order for
including your patches?
If the clean build still doesn't work - we'll find out together why ;-)
Ryan, thanks for the positive feedback about another target... :-)
Forget my complaint. It seems that it is now working in one of the latest builds. At least I got it to work.
Thanks for the patch, good work!
Norbert
The fact that this functionality is not in the official build is the only reason why I don't use them. And it's not like i didn't try to patch but Cygwin will not compile any build i edit, and in any case, with rockbox's generally short battery life charging is cruicial.
Does anybody else have some problems with that patch?
If yes, with which targets?
(it still does work here for my H300)
apps/settings.c
Hunk #1 FAILED at 331
Might be conflicting with another patch I have, the brightness patch
rather than a hidden sector on the HD it used previously.
I tried figuring out what needs to change but got completely lost in the new code. This patch is so essential. Anyone know if there's any chance of it ending up in the codebase?
Unfortunately, now I understand how USB connection works (i.e. this just inverts the holding down/not holding down a button test) I'm not sure that this patch is needed; it is considered option bloat (see IRC logs for discussion). Maybe someone can argue better for it's inclusion here...!
If the options are tucked away in well-organized menus, why is it a problem if there are many of them? I never use crossfade, but it doesn't bother me seeing it there in the menu. IMO, the more things Rockbox can do, the better.
Since Rockbox seems to be all about making good software for as many different devices as possible, why shouldn't it be as configurable as possible? Different devices and usage habits require different customization. This patch may be irrelevant to Archos users (I have no idea how they handle USB), or even iPod users who don't plug in several times a day. But to me as an iPod user who constantly plugs and unplugs the device, this patch is absolutely essential, and I think many other users would agree.
Or is code size the real concern?
Is code size a concern because Rockbox supports targets with very little memory? If so, given the number of #IFDEFs I already see in the code, couldn't an argument be made for conditional compilation of options like this too? It seems strange to deny useful features to those of us with a 30GB hard drive for fear of adding a few KB to the firmware.
I also think this is an important option though.
@pondlife: a big thanks for adding my other charging USB patch to SVN :-)
(the original version was written by pyro, but I rewrote it with suggestions from amiconn)
My two cents about the "code size concern" and "option bloat": I also follow MisterScoundrel's arguments. I never used crossfade (and other options, too) and I really don't see any essential need for some of those.
Each developer (including me) thinks that "his" feature and "his" options are the most important ones...
So why not have a complete (enduser) discussion about which options are "essential" and which are not?
BTW, for me some of the crossfade options could be removed in favour of these one here ;)
As for the portion of it changing the default behaviour while in car adapter mode, I think that should be separated into its own topic for discussion. In reality they're two different things.
-I think that although this doesn't actually involve playback, to some it might make a great deal of difference in the daily use of their device. Example: An individual who has his player at work and wants to plug it in to his computer to charge and play directly because he can't be playing music on that computer. I just think the simplicity of the option when compared to the benefit of the option is definitely a plus.
-Also, just for argument's sake, when asking the end user his opinion- the iPod population that uses Rockbox is quite different from the general iPod population.
As I have corrupted my nano's file system on more than one occasion by rebooting into disk mode by accident when I am trying to charge my ipod then ether unplugging it without stopping the removable disk under windows or when my laptop suspends.
So as such I think the default behaviour should ether be to require a key to be held down for USB mode. I know that a need to hold down the menu button when connecting but sme times I get it wrong.
Remember ipods are supplied without any other form of charger and most people plug them in to a USB port on a computer to charge much more often than to transfer music on or off then.
The first is "Add option to reverse the behaviour of the USB mode, so that holding a button enables it rather than preventing it."
The second is "Car mode by default prevents USB connection."
This task needs to be split into two separate tasks, as there are very likely many people who might agree with one and object to another (for example, me). I will close this task in relatively soon, please split into relevant tasks. We do strongly and frequently suggest that you do not make patches that do more than one thing, this patch clearly changes two behaviours.
Crossfade was only an example (although I - and it seems others too - personally never used it)
I'm sure there are a lot of other features which some wants and others not.
I also don't see the point against asking end users.
If they don't share the opinion of the developers, what's wrong with that?
It would show that end users have a different view about what features are bloated
and which are missing. Devs still have the choice to follow that views or not...
But it's a chance to see what a majority of end users think.
BTW, it seems that the (unrepresentative) people here have the opinion that the
default for USB connection should be to charge instead of to connect.
So why not ask the "end user" about that?
About IRC: when working on the album art I often went to IRC and asked for
suggestions/opinions. Mostly I spent a lot of time there and don't get any answers.
It seems that the hours where I was there didn't fit to times where core developers
had time to talk to me :-(
At the moment I don't have the time to spent hours at the IRC, so I don't do it.
@darkkone: I'm not sure if I'll find time and mood to split the patch
(and maintain two patches - as the past shows sometimes for a long time).
This patch here is alive for half a year now - in that time you're the first who
asked for a split and tries to force me to do it with the menace to close it.
If you're in the mood to close this patch here do it;
I can live with that...
As I see if this task has nothing to do with car adapter mode and is very much about audio playback as it allows the user to change the default behaviour of rockbox to continue audio playback when you plug in as USB cable.
As I have already stated all iPods are supplied without chargers and the most common reason for plugging usb into a rockboxed ipod is to charge it and I suspect there are quite a lot of people like me who still use there mp3 players for audio playback when plugged into USB to charge rather than using a computer.
Generally speaking, patches that do two things are less likely to be included because if the developers aren't fond of either thing it's likely to keep both out. I'm not simply threatening to close it. It's just that as the patch stands it falls in the 'rejected' category, so it'll be closed for that, but the only thing that's really come up for discussion is the configurable button.
I don't intend it as a threat, merely that the discussion has come and gone on one aspect but that's not all there is to the patch.
The patch description specifically includes a change to car adapter mode behaviour. If you're choosing to ignore that, it's fine, but as you can clearly see by actually reading the full description, the patch does in fact change car adapter mode.
Sorry I read the "additional" part of the description as a bug fix to existing functionality not an additional feature when I first read the description, why would any one want to reboot when they plug the power in car adapter mode?
But I have to admit that I had complete forgotten about that part of the patch as I have been using it for so long and I have never used car adapter mode. It is also not mentioned in the title “Button press for USB connection or USB power only”.
If this patch is being dropped I guess I will just have to order a plugin USB charger for my iPod now Rockbox supports them.
And the 'bug fix' one is, I think, more likely to get a yes, while the other one falls under the general category of 'not terribly useful.' All it does is change it so that, while you're already holding the player, you don't have to press a single button. Why that's worth adding yet another small amount of code to the pile is a mystery to me. It's a single button, one that I have no problem pressing while plugging in my players that can USB charge. It doesn't add a 'feature' it just very, very, very, very slightly makes Rockbox more convenient.
And while the amount of code size is trivial as well, it is non-zero. Remember that several people said 'I don't use crossfade'. Were this option to go in, people would surely start saying 'Well, I don't use the USB charging reverse option' when their small features are challenged. It adds next to nothing, and it also opens a door for a lot more argument on other issues.
If you can give me a solid reason why holding down a single button is such a huge problem that it needs code to fix, maybe there's something more here. But outside of 'in the car when only one hand is available' (which doesn't really count since A) You shouldn't be plugging it in once you've started driving, and B) the other patch that should be split from this would cover it), what situations are there where a single button press is such a burden that it should in _any_ amount decrease audio buffer for everyone?
I created an additional (small) new patch which just switches
on USB power when in car adapter mode (without choice).
It's FS#6654.
btw, this is out of sync.
And of course, because of the battery consumption I really liked the idea of letting the Ipod playing with USB power.
Maybe rockbox should be more modularized than. I could enjoy unloading the crossfade stuff to increase my audio buffer ...
the ipod video's lack firewire communication..however, since the firewire power pins have pretty much been designated as the main power input on the dock connector (they're tied to a handy voltage regulator)...they function just fine as power. now, up till a couple of weeks ago all i had was my USB cable...mostly because it came with the unit...and that was my main method of charging it...now...everyone should know USB is 5v..which sounds fine...but it's 500ma at most...which isn't a whole lot of juice for charging the battery....and with the recent purchase of an ipod travel kit...i got a firewire cable (the car adapter actually requires it)...now, since the 5th gen video's don't have firewire in it....all rockbox can do is go into charging mode...personally, i plug mine into the daisy-chain port on my external firewire drive....provides a good bit of power to charge the ipod.
now, naturally...much like this patch...it's a solution not everyone will agree upon..however, i thought i'd just..share in case anyone else wanted to start doing the same thing.