Measurements widget

[srcejon]

Hi Edouard,

This patch displays the spectrum measurements in a dockable widget, and with the extra space, I've expanded the measurements:

I've got a few more things I want to do, but I thought I'd ask for your feedback before going too far.

In order to make the widget dockable (so can be positioned underneath or to the side as in the above picture), and so only one object needs to be instantiated, I've created a GLSpectrumTop widget (Feel free to suggest a better name), that instantiates GLSpectrum and the SpectrumMeasurements (The widget that has the tables with the measurement results). This means that classes that want a spectrum should instantiate GLSpectrumTop rather than GLSpectrum (although it still works if they don't). In the patch, I've done this change for DeviceUISet/MainSpectrum and the ChannelAnalyzer.

(GLSpectrumGUI could possibly be instantiated in there as well, to combine all the spectrum GUI elements)

I was hoping you had a caliper icon :)

[f4exb]

This was not really my idea to have a different kind of spectrum that supports measurements. I think the spectrum component should support measurements just like it supports markers and thus the measurements widget would in fact be in the form of a dialog. It also allows to have both the table displays and controls in the dialog and thus not encumber the main spectrum controls. This in turns requires extensive changes in each plugin that wants a spectrum supporting measurements and also the main deviceUI. I think any component that uses the spectrum as sub component should benefit from it. Moreover I am not in favor of having dockable widgets inside widgets.

However I will compile your branch and see how it looks like to make myself a better opinion.

[srcejon]

I don't think separate windows (either non modal dialog or undocked widget) work particularly well with MDI, as I want the results next to the corresponding spectrum, but that's perhaps personal preference. With QDock the user can choose which they prefer, with a dialog they can't.

The changes needed to have any plugin support this aren't really significant. It took ~5 minutes to change the channel analyzer.

[f4exb]

Sorry I really do not like this design in particular having the QMainWindow in the GLSpectrumTop component. This is against Qt general design where QMainWindow is THE main window. I cannot accept this.

[f4exb]

The changes needed to have any plugin support this aren't really significant. It took ~5 minutes to change the channel analyzer.

Sorry all plugins having a spectrum should inherit the measurement feature it should be part of the spectrum functionality.

[srcejon]

Sorry all plugins having a spectrum should inherit the measurement feature it should be part of the spectrum functionality.

It is. They just need a small change because there's an extra widget. You already have GLSpectrum and GLSpectrumGUI. As mentioned, it could be written so just one widget needed to be instantiated.

[srcejon]

Patch updated to use a QSplitter instead of a QDock. Aside from not being able to move it, it doesn't seem to resize automatically like the dock did, but maybe just an extra constraint needed somewhere.

[srcejon]

As an example of why I'm not keen on separate windows, consider the Channel Analyzer and Demod Analzyer - if the results were in a separate window underneath - then you'd have the scope in the way - which in my opinion, isn't what you want when using the Peak table. It should be right next to the spectrum.

[f4exb]

if the results were in a separate window underneath - then you'd have the scope in the way

If you focus on spectrum measurements then you are probably not interested in the scope. You would even probably minimize it to get more space for the spectrum. And anyway you could move the separate window.

I don't think the measurements functionality deserves an extensive change on the spectrum component (just like the markers with which it should be on par) and thus opening a separate window like a non modal dialog (the markers dialog is effectively modal but this should be fixed) is in my opinion better because much less intrusive. In any case it should not be necessary to replace GLSpectrum by GLSpectrumTop wherever GLSpectrum is used. Instead if going this way the present GLSpectrum should be made a sub component of a new GLSpectrum which again I think is not desirable for the overall design of the application.

[srcejon]

If you focus on spectrum measurements then you are probably not interested in the scope. You would even probably minimize it to get more space for the spectrum. And anyway you could move the separate window.

You could make someone jump through those hoops, but I see no reason to, given that we can already place the results next to the spectrum, which is the most sensible location given the two are related.

I don't think the measurements functionality deserves an extensive change on the spectrum component (just like the markers with which it should be on par) and thus opening a separate window like a non modal dialog (the markers dialog is effectively modal but this should be fixed) is in my opinion better because much less intrusive.

I don't see the markers as being equivalent in use. There, a dialog makes sense, as you do the setup for your markers once, and can then close it, as it's not really needed anymore. This is unlike the measurement results, which should be displayed continuously. If you want the measurement settings in a dialog, then OK.

Note that one additional measurement setting I was considering, was being able to display the values of the markers in the tables (to get the precise values and averages for tracking markers)

In any case it should not be necessary to replace GLSpectrum by GLSpectrumTop wherever GLSpectrum is used. Instead if going this way the present GLSpectrum should be made a sub component of a new GLSpectrum

That's straightfoward enough - it's just different names.

[srcejon]

I've updated the SpectrumGUI to use a Dialog for the Measurements Settings, so there is only the single measurements button in the flow layout:

As a QDock is no longer used, I've added an option to specify placement of the results, below or to the side.

With the extra space available, ValueDials are used instead of SpinBoxes, which are a nicer way or adjusting the bandwidth.

[srcejon]

Latest patch has changed GLSpectrum to GLSpectrumView (quite a few Qt classes use the suffix View - I guess from MVC) and GLSpectrumTop is now GLSpectrum. This means that all plugins now support Measurements without any changes (and I've undone the changes to Channel Analyzer and MainSpectrum).

This approach does require a level of indirection in GLSpectrum. If this isn't desired, then the plugins could be changed to use m_spectrumView-> (initialised with m_spectrum->getSpectrumView()), instead of m_spectrum->.

[f4exb]

Latest patch has changed GLSpectrum to GLSpectrumView (quite a few Qt classes use the suffix View - I guess from MVC) and GLSpectrumTop is now GLSpectrum

Yes for me also the naming makes more sense and it is much better that GLSpectrum be maintained as the top component.

This approach does require a level of indirection in GLSpectrum. If this isn't desired...

I think this is pretty much desired so that the clients of the GLSpectrum component do not need to be updated.

My original idea of having everything (controls and displays) as a separate window was to minimize changes to GLSpectrum but this implementation is definitely more acceptable than the original. Having the controls in dialog is also better as as you noticed it gives more space for more suitable control widgets.

I've dig a bit the subject of having the QMainWindow inside a QWidget and in fact there does not seem to be strong arguments against it: https://forum.qt.io/topic/31841/qmainwindow-inside-a-widget-can-be-done-or-is-not-correct QMainWindow being a specialized kind of QWidget in fact the "main" term may be misleading. However as you said "I want the results next to the corresponding spectrum" so it probably makes more sense to use the QSpliter so thet the measurement display is an integral part of the whole thing (spectrum). It is rather pointless to have it wandering elsewhere.

Note that one additional measurement setting I was considering, was being able to display the values of the markers in the tables

There's a kind of conflict of interest with the markers. The peak thing should be a job for the markers and when you consider channel measurements it involves also some kind of (different) markers having some width like the annotation markers but used for instrumentation. When you do adjacent channel measurements you would normally use a symmetric structure as the one you developed but maybe sometimes you would like to have arbitrary in channel and out of channel definitions. Such "range" markers would allow this. So maybe at some point markers and measurement dialogs could be merged but that's OK for now.

For the cosmetic part I'll take care of differentiating measurement and calibration icons

[srcejon]

so it probably makes more sense to use the QSpliter so thet the measurement display is an integral part of the whole thing (spectrum). It is rather pointless to have it wandering elsewhere

Yeah. Having position of the widget set from the dialog is flexible enough. One thing the QDock did do better, was that it automatically sized the dock to the table size (unless it has been manually sized), which the QSplitter doesn't. So I will try to add something similar, so that the splitter allocates a more reasonable initial size to the table, which it isn't doing at the moment. Qt's size constraints are a bit of a mystery!

Also, I've seen a bit of a strange interaction with the Auto Stack layout when resizing the channels, which I will try to sort out.

For the cosmetic part I'll take care of differentiating measurement and calibration icons

Thanks.

[f4exb]

I have re-organized the documentation a little bit but keeping the same text of course.