drowe67 / freedv-gui

GUI Application for FreeDV – open source digital voice for HF radio
https://freedv.org/
GNU Lesser General Public License v2.1
193 stars 49 forks source link

Show green line indicating RX frequency. #725

Closed tmiw closed 2 months ago

tmiw commented 3 months ago

Implements request from @barjac to indicate the actual RX frequency (red line + averaged FreqOff over previous 2 seconds).

barjac commented 3 months ago

Thanks - looks good. :) I have tested using old recordings and it's working well. The running average timing is working as I expected and avoids excessive jitter.

Is it possible to reduce the length of the red marker so that some of the green one can still be seen when the frequencies match?

tmiw commented 3 months ago

Actually, I wonder if it'd be better to not show the green line unless the offset exceeds a certain amount since the purpose is to catch excessive drift, not normal drifting as a result of band conditions/normal radio operation. Thoughts?

barjac commented 3 months ago

Actually, I wonder if it'd be better to not show the green line unless the offset exceeds a certain amount since the purpose is to catch excessive drift, not normal drifting as a result of band conditions/normal radio operation. Thoughts?

Not keen on that. It is useful to watch the green line while tuning the xit/rit until the red and green align. If the green line was to vanish close to netting it could be really frustrating. :(

This is why I would prefer that the green did not totally hide behind the red when netted.

Shortening the top of the red line by maybe 1/3 of its current length would be perfect.

It could be useful to have the green turn orange when sync is lost - just a thought :)

tmiw commented 3 months ago

Shortening the top of the red line by maybe 1/3 of its current length would be perfect.

How about now?

It could be useful to have the green turn orange when sync is lost - just a thought :)

Done, but I'm not sure orange pops out enough if that makes sense:

Screenshot 2024-06-11 at 12 49 29 PM
Tyrbiter commented 3 months ago

Just tried this and although I have not yet found off-air signals to play with the green line indication, I wonder if the LIGHT_YELLOW colour might be a bit more contrasty against the red line. It's already defined so it only needs a one line change to plot_waterfall.cpp to try it out @barjac

barjac commented 3 months ago

Just tried this and although I have not yet found off-air signals to play with the green line indication, I wonder if the LIGHT_YELLOW colour might be a bit more contrasty against the red line. It's already defined so it only needs a one line change to plot_waterfall.cpp to try it out @barjac

I will take a look at the code but not just now.

I don't think yellow would be much use on a white background. I used to use a dark system theme for FreeDV but it upset some other programs so I reverted to a light one. Maybe if there was a 'Dark Mode' for FreeDV (as there is for klog (Qt based)) then it would not affect other programs.

In testing I find the orange OK for both light and dark, which was my reason for suggesting it.

If I move the red away from the green to about 60Hz offset then there is a point where decoding stops but the RX line remains green, which seems odd. I obviously don't understand just what is controlling the green/orange switch-over.

My original suggestion of a switch to orange was to have an indication on screen as to one reason why a station was possibly not decoding, despite having a strong signal, so I was expecting that the switch to orange would coincide with the decode stopping due to excessive frequency offset.

barjac commented 3 months ago

I would really like a hot key combination that re-centres the red line on 1500. It really is a pain to get it back on spot once moved. :) Unless there already is one that I missed.

Tyrbiter commented 3 months ago

I'm not sure about the sync thing, I can see where the sync_ variable is made equal to sync, but I am not sure if this is currently a one off event or if it follows the sync state in real time. Maybe @tmiw can comment, I'm not very C++ literate and on top of that event-driven programming ties my brain in knots.

Tyrbiter commented 3 months ago

I have now found the omission, this green/orange line works on the waterfall only, not on the spectrum plot which is what I usually look at. Mystery solved.

I notice that the coloured lines above the waterfall are wider than the tick marks on the frequency axis, the result is that the coloured lines are always 0.5 line widths to the left or right of the centre pixels, probably due to rounding in the freq to pixels conversion. Maybe making the line width = 3 would allow for centre-ing it accurately with the frequency ticks with the resulting slightly wider coloured lines being more obvious.

tmiw commented 3 months ago

I don't think yellow would be much use on a white background. I used to use a dark system theme for FreeDV but it upset some other programs so I reverted to a light one. Maybe if there was a 'Dark Mode' for FreeDV (as there is for klog (Qt based)) then it would not affect other programs.

Theoretically FreeDV should be following whatever the dark mode setting is for the DE you're using. This might work better when using GNOME, though (it definitely seems to work for Windows and macOS FWIW).

In testing I find the orange OK for both light and dark, which was my reason for suggesting it.

Yeah, it seems to show up better once there's an actual signal it's following, so we'll keep it orange.

If I move the red away from the green to about 60Hz offset then there is a point where decoding stops but the RX line remains green, which seems odd. I obviously don't understand just what is controlling the green/orange switch-over.

I tried this with a pre-recorded file and it seemed to change over to orange as expected once I get the red line far enough away from the center.

I would really like a hot key combination that re-centres the red line on 1500. It really is a pain to get it back on spot once moved. :) Unless there already is one that I missed.

Not sure about a key combination, but could this be a menu item in the Tools menu maybe? Or maybe an actual button on the left hand side (kinda like the Resync button).

I'm not sure about the sync thing, I can see where the sync_ variable is made equal to sync, but I am not sure if this is currently a one off event or if it follows the sync state in real time. Maybe @tmiw can comment, I'm not very C++ literate and on top of that event-driven programming ties my brain in knots.

The waterfall gets the sync status every ~100ms from MainFrame::OnTimer in main.cpp:

        if (m_panelWaterfall->checkDT()) {
            m_panelWaterfall->setRxFreq(FDMDV_FCENTRE - g_RxFreqOffsetHz);
            m_panelWaterfall->m_newdata = true;
            m_panelWaterfall->setColor(wxGetApp().appConfiguration.waterfallColor);
            m_panelWaterfall->addOffset(freedvInterface.getCurrentRxModemStats()->foff);
            m_panelWaterfall->setSync(freedvInterface.getSync() ? true : false);
            m_panelWaterfall->Refresh();
        }

I have now found the omission, this green/orange line works on the waterfall only, not on the spectrum plot which is what I usually look at. Mystery solved.

Just added this to the Spectrum plot as well. I didn't realize that you can adjust the RX frequency there too.

I notice that the coloured lines above the waterfall are wider than the tick marks on the frequency axis, the result is that the coloured lines are always 0.5 line widths to the left or right of the centre pixels, probably due to rounding in the freq to pixels conversion. Maybe making the line width = 3 would allow for centre-ing it accurately with the frequency ticks with the resulting slightly wider coloured lines being more obvious.

Based on the code I thought the lines were only 1px wide. Would this change make the colored lines 3px or the tick marks so?

Tyrbiter commented 3 months ago

Latest changes now show green/orange line on the spectrum plot along with the red line, but the red line is now missing from the waterfall plot, for me at least.

I'm struggling with the line width issue, for some reason my screen magnifier doesn't want to work, I think it's probably yet another GNOME security change. I wanted to get a magnified image of what I see.

I think the tick marks are 1 pixel wide, the coloured lines seem to show as 2 pixels wide. In order to make them able to overlay the tick marks I thought 3 pixels would make them easier to see, but it depends if the rounding calculations are the problem. I wondered if this might be why @barjac is commenting on the difficulty of re-centreing the red line using the mouse.

Here is a screenshot which you can probably zoom into to see the pixel reality :)

Screenshot from 2024-06-13 15-01-09

I like the idea of a reset to 1500Hz button.

barjac commented 2 months ago

Thinking some more about this it would seem that it is asking a bit too much of user's mouse handling abilities to adjust at single pixel levels.

Maybe a new 'Fine tune' widget incorporating a 1500 reset button and an expanded scale tuning adjustment spanning perhaps 300Hz in total would really enhance usability for elderly operators with lower dexterity and/or sight issues.

The widget would not replace this PR as it is so far, but augment it as a 'zoomed in' addition.

Clicking on the widget scale would do exactly as clicking above the waterfall/spectrum but with much better accuracy.

Another point that I think needs changing is to replace the double click requirement for moving the red line with a single click. It is easy to single click while keeping the mouse still but much more difficult to double click without disturbing it's position.

Tyrbiter commented 2 months ago

I think that the double click is there to avoid an accidental single click causing detuning.

Now that I have realised that the coloured lines are 2 pixels wide it explains why I can't position it where it overlaps the tick marks. Hence my suggestion about 3 pixels wide, it's a bit bigger for older eyes and it can overlay with 1 pixel each side of the central pixel.

tmiw commented 2 months ago

Thinking some more about this it would seem that it is asking a bit too much of user's mouse handling abilities to adjust at single pixel levels.

Maybe a new 'Fine tune' widget incorporating a 1500 reset button and an expanded scale tuning adjustment spanning perhaps 300Hz in total would really enhance usability for elderly operators with lower dexterity and/or sight issues.

How much accuracy do we actually need in general? Do we need to make it possible to, say, center it on 1499 Hz? Or would it be okay if double clicking only adjusted it in e.g. 25 Hz blocks? If the latter, there might be some stuff we can do to effectively snap to one of those blocks on double-click (thus increasing the amount of leeway one has in doing so).

I think that the double click is there to avoid an accidental single click causing detuning.

Yeah, I don't think this is a good idea for that reason.

Now that I have realised that the coloured lines are 2 pixels wide it explains why I can't position it where it overlaps the tick marks. Hence my suggestion about 3 pixels wide, it's a bit bigger for older eyes and it can overlay with 1 pixel each side of the central pixel.

It actually was 2px wide and not 3; I missed the part of the code that controlled that. I just checked in a change to make it 3px.

Tyrbiter commented 2 months ago

Yes, 3px wide is nicer, for my eyesight anyway, and it looks symmetrical as the lines move around. Red line back on the waterfall too, I had missed the // that was left in.

I would say that allowing adjustment in 5 or 10Hz increments ought to be good enough, 25Hz seems a little coarse. Maybe using ctrl-wheel for a larger step? Maybe with the mouse wheel as the adjusting mechanism, and/or up and down keys?

Being honest I see few stations much off frequency these days, it's mainly those with older radios that don't have TCXO reference oscillators and rarely on 'digital' modes.

tmiw commented 2 months ago

I would say that allowing adjustment in 5 or 10Hz increments ought to be good enough, 25Hz seems a little coarse. Maybe using ctrl-wheel for a larger step? Maybe with the mouse wheel as the adjusting mechanism, and/or up and down keys?

Adjustment is now in 10 Hz steps and can be done both using arrow keys (may need to single-click inside the waterfall or spectrum plot first) or by using the mouse wheel.

tmiw commented 2 months ago

And there's now a Tools->Center RX Frequency option to reset it to 1500 Hz. :)

Tyrbiter commented 2 months ago

The reset menu item works fine, the mouse wheel movement seems to be about 30Hz per click, but I'm not certain of the exact number, 3 clicks is almost 100Hz. Frequency<->pixels conversion scaling thing?

For me the up/down keys don't do anything after a click on the spectrum or waterfall. Or after a double click.

tmiw commented 2 months ago

The reset menu item works fine, the mouse wheel movement seems to be about 30Hz per click, but I'm not certain of the exact number, 3 clicks is almost 100Hz. Frequency<->pixels conversion scaling thing?

For me the up/down keys don't do anything after a click on the spectrum or waterfall. Or after a double click.

Do the left/right keys work? The keyboard handling supports those as well as up/down. Also, it might be worth trying the non-numpad versions (or the numpad ones, whichever you haven't tried yet).

Tyrbiter commented 2 months ago

Left/right changes tabs, same as numpad left/right. Up/down no effect on numpad or arrow key group.

tmiw commented 2 months ago

Left/right changes tabs, same as numpad left/right. Up/down no effect on numpad or arrow key group.

Might depend on DE as it seems to work on macOS. Maybe @barjac can try?

Tyrbiter commented 2 months ago

Wonder where the key left/right code is? Perhaps it's different in some way.

tmiw commented 2 months ago

Wonder where the key left/right code is? Perhaps it's different in some way.

Line 362 of src/gui/controls/plot_spectrum.cpp (and plot_waterfall.cpp). The key codes it's looking for should be platform independent, or so I thought.

Tyrbiter commented 2 months ago

I see. However, when the number of averages field on the spectrum tab is focused then the keys are seen (I get key click sounds) but when it isn't the up/down keys do nothing and left/right moves tabs.

Maybe the key definitions for the tab selection in other source files mean things get confused.

Not sure if it matters, but in plot.cpp there is a left mouse button down and up event, but for right mouse button only a down event. The same is true in plot_waterfall.cpp and plot_spectrum.cpp but again I'm not sure whether it's important.

tmiw commented 2 months ago

I see. However, when the number of averages field on the spectrum tab is focused then the keys are seen (I get key click sounds) but when it isn't the up/down keys do nothing and left/right moves tabs.

Maybe the key definitions for the tab selection in other source files mean things get confused.

Not sure if it matters, but in plot.cpp there is a left mouse button down and up event, but for right mouse button only a down event. The same is true in plot_waterfall.cpp and plot_spectrum.cpp but again I'm not sure whether it's important.

Yeah, I can't use the arrow keys on macOS to adjust in the spectrum plot either when the # of averages is selected. I believe this is expected behavior, though, since the event handler is on the rendered plot itself and not any controls associated with it.

Tyrbiter commented 2 months ago

Yeah, I can't use the arrow keys on macOS to adjust in the spectrum plot either when the # of averages is selected. I believe this is expected behavior, though, since the event handler is on the rendered plot itself and not any controls associated with it.

I thought that, but what I don't understand is why the up/down arrow keys are locked out, perhaps it's because the left/right key codes are reused together with up/down. wxWidgets on the Mac seems rather better than on Linux.

I'm going to see if I can get something working, but in any case the frequency increment is >10Hz :)

Tyrbiter commented 2 months ago

Editing plot_waterfall.cpp and plot_spectrum.cpp as shown below:

// case WXK_LEFT:

// case WXK_RIGHT:

Gives me a working up/down arrow keys and mouse wheel, left/right arrow keys change tabs.

wxWidgets being awkward again.

barjac commented 2 months ago

Sorry for delay in commenting - I had a rig failure and needed to do a repair - all OK again now :)

I would say that allowing adjustment in 5 or 10Hz increments ought to be good enough, 25Hz seems a little coarse. Maybe using ctrl-wheel for a larger step? Maybe with the mouse wheel as the adjusting mechanism, and/or up and down keys?

Adjustment is now in 10 Hz...

Testing #e53ac. Its about 30Hz per notch here using the scroll wheel at present. However the double clickable 'resolution' appears to be about 10Hz.

...steps and can be done both using arrow keys (may need to single-click inside the waterfall or spectrum plot first)

Arrow keys doing nothing here either.

I think the 3px wide lines make it more difficult to be precise.

Maybe a 'zoom' toggle button to expand the displayed frequency scale to show only +/-200Hz full width, centred around 1500Hz could solve these issues? This would allow easy fine adjustment even with the 3px lines.

The Menu option to re-centre works fine but a menu option seems to be the wrong place - I think it needs a button somewhere.

tmiw commented 2 months ago

Editing plot_waterfall.cpp and plot_spectrum.cpp as shown below:

// case WXK_LEFT:

// case WXK_RIGHT:

Gives me a working up/down arrow keys and mouse wheel, left/right arrow keys change tabs.

wxWidgets being awkward again.

Removed WXK_LEFT and WXK_RIGHT in the latest changes so we can at least try to be consistent across platforms.

Testing #e53ac. Its about 30Hz per notch here using the scroll wheel at present. However the double clickable 'resolution' appears to be about 10Hz.

It should now be using 10 Hz for the wheel as well.

Maybe a 'zoom' toggle button to expand the displayed frequency scale to show only +/-200Hz full width, centred around 1500Hz could solve these issues? This would allow easy fine adjustment even with the 3px lines.

That would actually be a significant change, unfortunately. I'd like to avoid this if possible. It seems like making the colored lines 1px may help, but could also cause problems for the visually impaired too. We can revisit at some point if it turns out there's significant demand for a "zoom" type feature.

The Menu option to re-centre works fine but a menu option seems to be the wrong place - I think it needs a button somewhere.

The latest changes now make this a button under "ReSync" instead of a menu item.

barjac commented 2 months ago

OK this is looking good now - well done! :)

I used it during our Sunday net and found that it was easy to see my radio drift and correct it with xit/rit as time progressed.

Two things:

The scroll wheel does nothing on the first step after placing it over the frequency scale, subsequent steps work fine.

Could middle mouse click when over the freq scale be used to centre the frequency? It would be very intuitive since the middle button is normally the scroll wheel itself! I wish I had thought of that sooner /o\ :)

Tyrbiter commented 2 months ago

Two things:

The scroll wheel does nothing on the first step after placing it over the frequency scale, subsequent steps work fine.

I don't see that effect, first movement of wheel results in a 10Hz adjustment.

Could middle mouse click when over the freq scale be used to centre the frequency? It would be very intuitive since the middle button is normally the scroll wheel itself! I wish I had thought of that sooner /o\ :)

That's a good idea.

For the line widths perhaps a setting under options, with 1 and 3 pixels being available?

barjac commented 2 months ago

Two things: The scroll wheel does nothing on the first step after placing it over the frequency scale, subsequent steps work fine.

I don't see that effect, first movement of wheel results in a 10Hz adjustment.

I just double checked and no matter whether I left click in the widget first or not it takes a second indent of the wheel to start any movement of the red line.

Could middle mouse click when over the freq scale be used to centre the frequency? It would be very intuitive since the middle button is normally the scroll wheel itself! I wish I had thought of that sooner /o\ :)

That's a good idea.

For the line widths perhaps a setting under options, with 1 and 3 pixels being available?

That sounds good! 1px for millenials and 3px for pensioners :)

Tyrbiter commented 2 months ago

I just double checked and no matter whether I left click in the widget first or not it takes a second indent of the wheel to start any movement of the red line.

KDE vs GNOME? Wouldn't surprise me.

tmiw commented 2 months ago

Could middle mouse click when over the freq scale be used to centre the frequency? It would be very intuitive since the middle button is normally the scroll wheel itself! I wish I had thought of that sooner /o\ :)

Done.

KDE vs GNOME? Wouldn't surprise me.

I just tested this on both and the first indent seems to adjust by 10 Hz in both environments. I'm running dnf update on my KDE VM (Fedora 39) now in case a recent update messed this up.

For the line widths perhaps a setting under options, with 1 and 3 pixels being available?

That sounds good! 1px for millenials and 3px for pensioners :)

Is this something that would still be good to have considering all the different ways of adjusting/resetting the frequency that now exist? Or at least something that can be deferred until there's more demand?

Tyrbiter commented 2 months ago

For the line widths perhaps a setting under options, with 1 and 3 pixels being available?

That sounds good! 1px for millenials and 3px for pensioners :)

Is this something that would still be good to have considering all the different ways of adjusting/resetting the frequency that now exist? Or at least something that can be deferred until there's more demand?

I'm actually fine with the 3px width lines, and as you say we now have many options to reset, thanks @tmiw

Once this hits an actual release we could see if there are any comments.

tmiw commented 2 months ago

I would need to plug my external mouse back in to be 100% sure but after upgrading the packages on Fedora 39, I think I'm seeing the same issue @barjac was seeing when I try to use two finger scrolling with my laptop's touchpad. Unfortunately I'm not sure this is something freedv-gui can work around.

Anyway, I can merge once @barjac confirms that the mouse wheel click works for him. 👍

Tyrbiter commented 2 months ago

I would need to plug my external mouse back in to be 100% sure but after upgrading the packages on Fedora 39, I think I'm seeing the same issue @barjac was seeing when I try to use two finger scrolling with my laptop's touchpad. Unfortunately I'm not sure this is something freedv-gui can work around.

I am using Fedora 40, so it's more recent and has a later set of libraries for GNOME and KDE, that might make a difference.

Anyway, I can merge once @barjac confirms that the mouse wheel click works for him. 👍

It certainly works for me.

tmiw commented 2 months ago

I am using Fedora 40, so it's more recent and has a later set of libraries for GNOME and KDE, that might make a difference.

Yeah, just updated to Fedora 40 as well and I'm not seeing it on KDE either.

It certainly works for me.

Hmm, OK, I'll merge and I can always do another PR if needed.

barjac commented 2 months ago

Sorry for delay - been busy. Yes the centre mouse button click works fine. The scroll wheel still misses the first indent. I am using KDE plasma 6.1 beta which is the current in our dev branch (Cauldron for Mageia-10). Currently plasma-workspace-6.1.0.

tmiw commented 2 months ago

Sorry for delay - been busy. Yes the centre mouse button click works fine. The scroll wheel still misses the first indent. I am using KDE plasma 6.1 beta which is the current in our dev branch (Cauldron for Mageia-10). Currently plasma-workspace-6.1.0.

6.0.5 for me:

Screenshot 2024-06-20 at 9 54 09 PM

Maybe it's something related to the beta?