Skip to content

enh(cpn): refactor edit application preferences - #7623

Open
elecpower wants to merge 13 commits into
mainfrom
elecpower/cpn-refactor-app-settings
Open

enh(cpn): refactor edit application preferences#7623
elecpower wants to merge 13 commits into
mainfrom
elecpower/cpn-refactor-app-settings

Conversation

@elecpower

@elecpower elecpower commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What users will notice:

  • reorganised tab contents, collapsible sections and scroll bars so content fits on screen
  • renamed menus from settings to preferences to remove ambiguity with model settings
  • firmware type picker dialog grouped by manufacturer

Under the covers:

  • complete ui rewrite
  • store code files in own directory
  • eliminate monolithic code files
  • change code references from settings to preferences
  • extend auto widgets to support getters and setters for field values
  • new auto widgets for paths (to and from native format), push button and file, folder, colour selection
  • split OpenTx firmware settings string into separate settings fields
  • add manufacturer getter to boards (yes this means another list to be maintained
  • housekeeping)

TODO as separate PRs (as this is big enough):

  • convert settings to meaningful names eg gePath holds Google Earth executable path
  • refactor Firmware and Boards classes
  • add models folder setting globally and per profile
  • consider firmwaredefs.json to simplify maintenance of opentxinterface

Some sample side by side screenshots:
image
image
image
image

image

@elecpower elecpower added enhancement ✨ New feature or request companion Related to the companion software labels Aug 6, 2026
@elecpower

Copy link
Copy Markdown
Collaborator Author

Working on tree styled radio picker with radios grouped by manufacturer to replace the long dropdown combo box

@pfeerick

Copy link
Copy Markdown
Member

Oh, thanks Neil... wasn't going to look at that until next week :)

@pfeerick

Copy link
Copy Markdown
Member

Looking very nice, and should be good on my other two laptops given the much smaller screen footprint! Have only tried on macOS so far, and will take a bit of getting used to everything being collapsed by default... (although I wonder if that is actually strictly necessary since we can collapse on demand now 🤔 ).

It seems the simulator page is wider than all the others... is it worth getting the initial preferences dialog window size to be the same as that, so it doesn't resize as you change through the pages? Also, it forces the shrink even if I resized the size manually and re-open it... claude suggested this if that helps / is of interest?

Suggested fix: measure every tab's size hint once (each is invisible at this point in the constructor, so no flicker) and size the dialog to the largest, and only do that when there's no saved geometry to restore.

--- a/companion/src/prefsedit/prefs_edit.cpp
+++ b/companion/src/prefsedit/prefs_edit.cpp
@@ -41,7 +41,7 @@ PrefsEditDialog::PrefsEditDialog(QWidget * parent, UpdateFactories * factories
  ui->setupUi(this);
   setWindowIcon(CompanionIcon("apppreferences.png"));
   setAttribute(Qt::WA_DeleteOnClose);
-  restoreGeometry(g.prefsEditGeo());
+  bool hasSavedGeo = restoreGeometry(g.prefsEditGeo());

   PrefsProfilePanel *prefsProfPanel = new PrefsProfilePanel(this, firmware, board, profile);
   PrefsPanel *profPanel = addTab(prefsProfPanel, tr("Radio Profile"));
@@ -61,7 +61,9 @@ PrefsEditDialog::PrefsEditDialog(QWidget * parent, UpdateFactories * factories
   connect(prefsProfPanel, &PrefsProfilePanel::sdPathChanged, prefsUpdatePanel, &PrefsUpdatePanel::onSDPathChanged);

   ui->tabWidget->setCurrentIndex(0);
-  shrink();
+
+  if (!hasSavedGeo)
+    shrink();
 }

 PrefsEditDialog::~PrefsEditDialog()
@@ -158,7 +160,20 @@ void PrefsEditDialog::setMainWinHasDirtyChild(bool value)

  void PrefsEditDialog::shrink()
 {
-  adjustSize();
+  // adjustSize() only accounts for the currently visible tab, since
+  // QStackedWidget doesn't lay out hidden pages. Walk every tab once so
+  // each panel reports its real size hint, and size the dialog to fit
+  // the largest one so switching tabs later doesn't resize the window.
+  QSize maxHint;
+  const int current = ui->tabWidget->currentIndex();
+
+  for (int i = 0; i < ui->tabWidget->count(); i++) {
+    ui->tabWidget->setCurrentIndex(i);
+    ui->tabWidget->currentWidget()->adjustSize();
+    maxHint = maxHint.expandedTo(ui->tabWidget->currentWidget()->sizeHint());
+  }
+
+  ui->tabWidget->setCurrentIndex(current);
+  adjustSize();
+  resize(maxHint.expandedTo(size()));
 }

@pfeerick

Copy link
Copy Markdown
Member
  1. Hitting the close button on the preferences dialog will trigger a double save prompt if you discard 🤪

  2. It seems

// delete dlg or deleteLater() triggers segfault with no debug stack trace in Linux

is due to a double-delete

prefs_edit.cpp:42 adds setAttribute(Qt::WA_DeleteOnClose) and correspondingly, mainwindow.cpp:461 lost its dialog->deleteLater() - That's a double-delete, not a mystery — done() already schedules the deletion

  1. I think we have a gremlin hiding in the undoFirmwareChange reset prompt... i.e.
  • Open a model file and make an unsaved change
  • Open Preferences → Radio Profile → change the radio to an incompatible board
  • Click OK on the Preferences dialog
  • You get the "You cannot switch Radio Type or change Build Options while there are unsaved file changes" prompt. Choose Reset.

Claude is waffling about

profile.fwName("") is no longer cleared on radio change. Old apppreferencesdialog.cpp:222 did profile.fwName("") alongside profile.fwType(newFw->getId()). Nothing in prefsedit/ does. FlashFirmwareDialog's ctor initialises fwName(g.profile[g.id()].fwName()), so after switching radio type, Flash Firmware prefills the previous radio's binary path.

New code emits resetFirmware(), then falls through to Firmware::setCurrentVariant(firmware); fwchange = true; and on to panel->save() for every panel — so Reset now saves everything and emits firmwareProfileChanged() for a change that was just undone. And the postChanged lambda at line 74-76 does profile.generalSettings(QByteArray()); profile.timeStamp(QString()); — which the old dialog never did here. That wipes the profile's radio-settings backup the moment you pick a different radio in the picker, and undoFirmwareChange() cannot restore it. The Reset option no longer fully resets.

And I'm not awake enough to even attempt to process that 🤪

I'm getting random instances where the preferences panels seem to go blank on Windows when interacting with the collapsible headers), but I've not figured out a reliable repro for that yet, or tell if it is windows specific yet.

Might have some issues here also

  • mainwindow.cpp:424 Firmware * newFw = Firmware::getFirmwareForId(g.getProfile(pid).fwType());
  • mainwindow.cpp:1301 Firmware::setCurrentVariant(Firmware::getFirmwareForId(g.currentProfile().fwType()));

Some potential cleanup

  • runPreUpdate() and applyPreUpdate() are identical; only applyPreUpdate() is called
  • resetUpdatesSettings() calls updDelDownloadsReset() twice
  • autocolorselectbtn is orphaned

@elecpower

elecpower commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author
  1. not in my Ubuntu world. I actually found the dirty variable is always false when close clicked as opposed to reject which tests the value correctly. Yet the variable can be true before close clicked. So something is resetting it or the reference is to a 'random' memory address. Playing with setAttribute(Qt::WA_DeleteOnClose) and MainWindow::editPreferences dlg->deleteLater makes no difference. I did find one thread where there can be differences between linux and windows but whether that still has some degree of truth in the Qt version we are using is another story.
  2. makes sense.
  3. I've struck the blank collapsible headers but not consistent. If you switch tabs then by magic the widgets appear. So best guess its refresh event sequencing and animation.

Working through the other points...

@elecpower

elecpower commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

And the postChanged lambda at line 74-76 does profile.generalSettings(QByteArray()); profile.timeStamp(QString()); — which the old dialog never did here. That wipes the profile's radio-settings backup the moment you pick a different radio in the picker, and undoFirmwareChange() cannot restore it. The Reset option no longer fully resets

Yeah I caught that but since I'm lazy and have been working on this for some time thought 'well if you change the type then fool on you' but the inconsistency needs fixing.

@elecpower

Copy link
Copy Markdown
Collaborator Author

So I solved my dirty variable issue.

Steps to recreate:

  1. open Preferences
  2. change profile name and do not click anywhere else
  3. close window

Result dirty not updated and no prompt to Save

  1. re-open Preferences
  2. change profile name and click anywhere in the dialog
  3. close window

Result dirty was updated and prompt to Save pops up.

@elecpower

Copy link
Copy Markdown
Collaborator Author

And the postChanged lambda at line 74-76 does profile.generalSettings(QByteArray()); profile.timeStamp(QString()); — which the old dialog never did here. That wipes the profile's radio-settings backup the moment you pick a different radio in the picker, and undoFirmwareChange() cannot restore it. The Reset option no longer fully resets.

Fix defers clearing until save confirmed

@pfeerick

pfeerick commented Aug 21, 2026

Copy link
Copy Markdown
Member

So I solved my dirty variable issue ... do not click anywhere else

Oh, is that all you had to do... just not click anywhere... 🤣 🤭

well if you change the type then fool on you

Expecially if you do it that way... but hey... I would have also liked it to have been a big red threatening button ... that does nothing when you press it ... you were half-way there 🤣

@elecpower

Copy link
Copy Markdown
Collaborator Author

profile.fwName("") is no longer cleared on radio change. Old apppreferencesdialog.cpp:222 did profile.fwName("") alongside profile.fwType(newFw->getId()). Nothing in prefsedit/ does. FlashFirmwareDialog's ctor initialises fwName(g.profile[g.id()].fwName()), so after switching radio type, Flash Firmware prefills the previous radio's binary path.

Fixed

@elecpower

Copy link
Copy Markdown
Collaborator Author

New code emits resetFirmware(), then falls through to Firmware::setCurrentVariant(firmware); fwchange = true; and on to panel->save() for every panel — so Reset now saves everything and emits firmwareProfileChanged() for a change that was just undone.

The rework should fix this I think...

@elecpower

Copy link
Copy Markdown
Collaborator Author

Some potential cleanup
runPreUpdate() and applyPreUpdate() are identical; only applyPreUpdate() is called
resetUpdatesSettings() calls updDelDownloadsReset() twice
autocolorselectbtn is orphaned

Fixed

@elecpower

elecpower commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Might have some issues here also
mainwindow.cpp:424 Firmware * newFw = Firmware::getFirmwareForId(g.getProfile(pid).fwType());
mainwindow.cpp:1301 Firmware::setCurrentVariant(Firmware::getFirmwareForId(g.currentProfile().fwType()));

These are ok in their context.

@pfeerick this completes my review and fixes for this round of feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

companion Related to the companion software enhancement ✨ New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants