BatchDrake / SuWidgets

Sigutils-related widgets
GNU General Public License v3.0
12 stars 12 forks source link

exchange mid & left mouse button. #11

Closed jedi7 closed 3 years ago

jedi7 commented 3 years ago

Hi, I switched the btns left & middle for mouse on Waterfall spectrum.

Previous behavior:

New behavior:

Reason:

BatchDrake commented 3 years ago

Hi Jaroslav, and thank you for your PR!

The problem I see with this change is that is going to break the current behavior (which could be confusing for current users) and it is also going to be surprising for new users coming from Gqrx (which is rather popular in both Mac and GNU/Linux).

However, I see a way this could work. How about adding methods to this class to enable and disable this behavior programmatically (e.g. setDragBehaviorReversed(bool) and getDragBehaviorReversed(), defaulting to disabled) and adding a checkbox in SigDigger's FFT panel to toggle this behavior?

jedi7 commented 3 years ago

Hi, you are right, can you please point me, where will be good place to put this check box? I'm still not so much oriented in the project.

BatchDrake commented 3 years ago

The process is a bit tricky because Ui components are highly decoupled between them, but on the other hand, we have a procedure for this. The Waterfall widget is used by the MainSpectrum component:

On the other hand, the FftPanel component is implemented here:

Both components are fundamentally independent of each other, and they don't (and should not) know about each other either. Instead, there is a mediator class called UIMediator:

which owns all components and is in charge of reacting to events happening in them. So your roadmap would look a bit like this:

Phase 0: prerequisites

Make sure you are working on branch develop, both in SuWidgets and SigDigger.

Phase 1: add the checkbox to SigDigger's FftPanel

  1. Edit FftPanelConfig in FftPanel.h and add a bool member named reverseDragBehavior, defaulting to false.
  2. Edit FftPanelConfig::serialize and FftPanelConfig::deserialize in FftPanel.cpp so it is able to save and restore the reverseDragBehavior flag accordingly.
  3. Add the checkbox. You should open the .ui file with either QtDesigner or QtCreator (in fact, it may be convenient to open the .pro file directly from QtCreator). Give it some descriptive name like reverseDragBehaviorButton.
  4. Save and regenerate the code. You can do this from QtCreator directly or just by running make on the command line.
  5. Add members to FftPanel named setReverseDragBehavior and getReverseDragBehavior. Check out other setters and getters in the class to see how they are implemented.
  6. Modify FftPanel::applyConfig(void) to change the status of the checkbox as soon as the config is applied to the UI.
  7. Add a signal in FftPanel.h named reverseDragBehaviorChanged() and a slot named onReverseDragBehaviorChanged(). The slot should be implemented in FftPanel.cpp at the end of the file, and should emit the signal reverseDragBehaviorChanged(). You can check other implementations of slots in the file to get some inspiration, most of them follow basically the same structure.
  8. Connect the clicked(bool) signal of your checkbox to the slot onReverseDragBehaviorChanged() in FftPanel::connectAll in FftPanel.cpp

Phase 2: add a setter to SigDigger's MainSpectrum

  1. Edit MainSpectrum.cpp to add a setter named setReverseDragBehavior(bool) that configures the Waterfall (this->ui->mainSpectrum) reverse drag behavior, according to its argument.

Phase 3. connect both components together

  1. Edit UiMediator.h to add a slot named onReverseDragBehaviorChanged() (Ctrl+F // Fft Panel in the file to find a good place to put it)
  2. Edit FftMediator.cpp to implement onReverseDragBehaviorChanged() by inspecting the this->ui->fftPanel->getReverseDragBehavior() you implemented earlier in FftPanel and calling this->ui->spectrum->setReverseDragBehavior(bool) accordingly.
  3. Edit UIMediator::connectFftPanel() to connect the FftPanel's signal reverseDragBehaviorChanged() to UIMediator's slot onReverseDragBehaviorChanged() you've just written.

And that should be it! :) Don't hesitate to ask me if you have any questions, I know this is not trivial and the project is a bit big already.

jedi7 commented 3 years ago

Thank you for the guide, it is very helpful :) What about to put the config into the Settings? I can create new tab "GUI" and put the check box here, will it be ok? Because this settings user set only once, so why take place in FFT, when it can be in settings :) Are you ok with that?

BatchDrake commented 3 years ago

That would be ideal, actually. I just didn't want to overload you with instructions.

Anyways, if you want to do that, you would have to edit the dialog in ui/Config.ui, then the corresponding source file in Components/ConfigDialog.cpp, and then add a method to ConfigDialog to retrieve the UI config. The problem with this is that there is no class representing the UI config, just something called ColorConfig that is retrieved through ConfigDialog::getColors() (which, in turn, is used to store it in AppConfig which holds configurations like the FFT panel config, the audio config, the inspector config, etc). Maybe it makes sense to create a serializable class called UITweaks that controls all these small behavioral details of the application. Then, both UIMediator::onTriggerSetup(bool) and UIMediator::applyConfig() will have to be modified to read these tweaks, place them in the ConfigDialog and apply them appropriately. Also, AppConfig has to be modified to trigger the serialization and deserialization of these UITweaks.

PS: if you play with this, you will find that clicking OK in the config dialog, even if you didn't change any source-related setting, will restart the capture. You are not doing anything wrong, this is a limitation that exists due to laziness by my side. Ideally, ConfigDialog should detect changes in every component and flag these changes in a boolean value, so the caller knows whether it makes sense to restart the source or not, change the colors, tweak anything, etc. But that's another story.

jedi7 commented 3 years ago

Hi, I created new PR for this. Can we continue there? https://github.com/BatchDrake/SigDigger/pull/137