Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10747 - Disable actions if rbutil is not configured properly

Attached to Project: Rockbox
Opened by Tomer Shalev (tomers) - Saturday, 31 October 2009, 10:25 GMT+2
Last edited by Tomer Shalev (tomers) - Sunday, 01 November 2009, 18:16 GMT+2
Task Type Patches
Category Rbutil
Status Closed
Assigned To Tomer Shalev (tomers)
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch disables all actions if rbutil is not configured yet.
It also adds disabling of menu items which were not disabled whenever their corresponding buttons got disabled.
This task depends upon

Closed by  Tomer Shalev (tomers)
Sunday, 01 November 2009, 18:16 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Committed in r23466
Comment by Dominik Riebeling (bluebrother) - Saturday, 31 October 2009, 11:51 GMT+2
This doesn't work: bootloadermethod=none does *not* mean that it's not configure but that no bootloader is required. This is the bootloader method used for the archos line, and disabling installation in this case will disable installation for archos players.

Also, while the original idea is good I think this doesn't go far enough. If the configuration is wrong then there's no point in having the whole tab widget active at all -- if the configuration is invalid you can't install anything else either.

   rbutil_disable_tab_on_wrong_config.diff (0.9 KiB) 
 b/rbutil/rbutilqt/rbutilqt.cpp |    3 +++
 1 file changed, 3 insertions(+)

Comment by Tomer Shalev (tomers) - Saturday, 31 October 2009, 13:04 GMT+2
- Control both buttons and menu items enable state according to whether user configuration is valid.
- On load, set the selected tab to be the first tab, instead of the extras tab.
- Disable the whole tabs widget and Actions menu item if rbutil is not configured properly
   rbutil_disable_widget_when_not_configured.diff (3.7 KiB) 
 rbutil/rbutilqt/rbutilqt.cpp |   55 +++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

Comment by Tomer Shalev (tomers) - Saturday, 31 October 2009, 13:09 GMT+2
This patch does not contains the irrelevant remark line change slipped through previously.
   rbutil_disable_widget_when_not_configured.diff (3.4 KiB) 
 rbutil/rbutilqt/rbutilqt.cpp |   53 +++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

Comment by Dominik Wenger (Domonoky) - Saturday, 31 October 2009, 15:43 GMT+2
I like the idea of this patch. And i really applaud to tomers for helping out with rbutil.

But i think the condition for bootloader_removable is not correct. We should check the flags of the bootloaderinstall class to see if it can remove the bootloader. And not just check for fwpatcher or none as install method.
Comment by Dominik Riebeling (bluebrother) - Saturday, 31 October 2009, 17:01 GMT+2
I don't see the effect that the tab initially being active after startup is the Extras tab. Have you possibly modified the rbutilqtfrm.ui file? This file controls the default until explicitly set by the application. As this also generated code for setting the active tab it should be done in the ui file if possible.
Comment by Tomer Shalev (tomers) - Saturday, 31 October 2009, 21:53 GMT+2
Another try.
- Removed setting active tab (there was no such issue, afterall).
- Renamed status variables to better match their intention.
- Add TODO comment for further work that needs to be done with regard of reading bootloader capabilities from the bootloader's own capabilities flag, and not from the settings.
   rbutil_disable_widget_when_not_configured.diff (3.8 KiB) 
 rbutil/rbutilqt/rbutilqt.cpp |   58 ++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 27 deletions(-)

Loading...