Bugzilla – Bug 2540
Brightness dims when holding down button for firmware update
Last modified: 2008-09-15 14:39:24 UTC
The brighness dims while holding down the Brightness button to udpate firmware. This can lower the brightness level enough that the update progress cannot be seen. Perhaps brightness should be full strenght during an update?
*** Bug 2539 has been marked as a duplicate of this bug. ***
odd. on mine, it brightness does go to full when the update triggers. What do you have for the autobrightness setting? Are you starting from full brightness to begin with?
Autobrightness is set to auto. This happens on two players: SB2 and my new SB3. Brightness level at 4 on all three settings. Yes, brightness is set to full when starting the update. This has occurred for me with many versions of firmware (don't recall how far back), and with v6.1.2, all nightlies up through 6.2 and 6.5.
As of firmware 28, this bug still occurs. Holding down the brightness to start the update reduces the brightness to minimal level. It then kicks back up at the end of the update, but ultimately goes back to the minimal level. You have to manually increase the level again once the system has finished rebooting.
As of firmware 29, this problem might be resolved. My test was brief, and not exhaustive. I downgraded to firmware 28, and in the process, the brightness level remained the same throught the update progress bar to reboot and Now Playing. Then I needed to upgrade to 29 again (from 28), and the progress bar dimmed. Next time I upgrade (to 30 I suppose) I will confirm the fix.
Nope, still not resolved with 33.
*** This bug has been marked as a duplicate of 3289 ***
It seems that this bug is not the same as 3289, as triode sugggested. I just upgraded from 41 (where the fix for 3289 was reported) to 43, and the same brighness decreased occurred immediately upon holding down brightness. I'm reopening.
I'm fairly sure this has to do with these lines in Squeezebox::upgradeFirmware_SDK5: $client->prefSet( "powerOnBrightness", 4); $client->prefSet( "powerOffBrightness", 1); $client->brightness($client->prefGet($client->power() ? 'powerOnBrightness' : 'powerOffBrightness')); This was added at change 4126 and change 4222 back in Sept 2005 for reasons I'm unable to guess. As these were done by moser and triode, maybe they can recall why and perhaps these are no longer required (I get the impression it was a pref handling changes that required a reset)
Agree with kdf - can't see why this could not be removed. I would suggest that the action should be to leave the brightness unchanged during an upgrade. The only case I can see where we may want to change it is if it is currently 0 (off). In this case I would want to set the brightness but not change any of the preferences - perhaps change to the minimum brightness level. My change 4222 was only to maintain previous operation when I removed the on change for the brightness preference - it should not have changed anything else. I think Moser's change only changed the pref API, not the action. The actual fixing of the brightness goes all the way back to 697 when upgradeFirmware_SDK5 was added!
It goes back even further than that, change 286 is where Dean first did the clientSet(power(On|Off)Brightness) in the previous firmware upgrade code, but that was just a change from using the vfdBrightness(MAXBRIGHTNESS) command, which was a change from using vfdBrightness(+1), which came in change 174 (also Dean). Sean at change 697 probably just copied the old firmware update and then made the changes to make it new. One thing I'd like to see is the output from a d_ir when this problem occurs. It would be interesting to see if some brightness.single events were happening during or after the update. Dean/Sean, if we send a brightness command to the player while it is updating, will it accept the command? I think the basic reason for sending the brightness command is that without a brightness command, the display will go to a brightness of 0 after the firmware update. There might also be some brightness button events in the IR queue for the player which get processed after the firmware update is finished. We should probably clear the IR queue for the updating player when we know that a firmware update was requested. Also, we should kill any Slim::Hardware::IR::checkRelease timers that have been set for that player.
Slim::Player::Squeezebox::reconnect will set the brightness at reconnect to the relavent pref value. I tend to think we should not be changing the prefs - if user sets the brightness to 0 in a mode we should keep this. One for Dean. My patch 6956 should avoid any queued IR brightness commands doing changing any preferences, but agree we should probably ditch them. [actually my change is useless as it is due to the issue here!] If Dean can confirm a view on removeing the brightness setting during upgrade, I'm happy to look at this and the IR bit.
The text that's displayed while downloading (with the progress bar) is just from the server, so you should be able to put brightness commands in there. BUT: writing each firmware block takes a long time and what's actually going on is that we queue up the whole series of firmware blocks, interleaving them with display updates, so as each block gets written, the progress goes up. Then when it's done, the player reboots, so sending any brightness commands after that long queue would be ignored. I think that we just need to override the brightness ir button handler when in the blocked "Press and hold..." mode so that the server never actually gets the brightness commands at all and so we don't need to do any crazy overriding of preferences, etc... We'll probably want to send a special one-time brightness command upon entering the blocked mode to make sure that the upgrade message is visible.
Please try svn r7199. Should only change brightness if necessary to see progress bar and will not save it across a reboot. Should trap all cases of brightness being presses during upgrade to avoid changing the brightness prefs.
Sounds like this is fixed then.. Please reopen if it's not. Thanks