Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multiple independent bandmap windows #593

Closed
wants to merge 4 commits into from
Closed

Multiple independent bandmap windows #593

wants to merge 4 commits into from

Conversation

kyleboyle
Copy link

@kyleboyle kyleboyle commented Feb 7, 2025 •
edited
Loading

closes #339

Attempt at adding multiple bandmap windows, using the existing right click menu to change which band is displayed. The VFO linked bandmap serves as the "master" which manages the spot data model shared with the non-vfo bandmaps. The user may close the vfo bandmap but it continues to operate in the background as it always has. This master vfo bandmap is also used to launch new non-vfo bandmaps and continues to provide the existing clear all and spot expiry features. I've tried to reuse as much of the existing bandmap as possible for the non-vfo bandmaps. Basically they are stripped of rig link and data management.

This change introduces a new concept around dockwidgets, which is the actual deletion and removal of dockwidgets when they are closed. This goal so far has been unexpectedly buggy, which is why you see some weird management code in mainwindow, even though this feature is meant to be built into qt but it doesn't work in many different scenarios. (WA_DeleteOnClose)

There are some remaining items to clean up which I will continue with if @foldynl gives the go ahead for this feature.
Screenshot 2025-02-06 at 9 22 35 PM

Kapture.2025-02-06.at.21.41.34.webm

(I gave up working on split mode support, too many design choices to do on my own)

@kyleboyle kyleboyle marked this pull request as draft February 7, 2025 01:50
@aa5sh
Copy link

aa5sh commented Feb 7, 2025

This looks great @kyleboyle

@foldynl
Copy link
Owner

foldynl commented Feb 7, 2025

I have tried several concepts on how to implement this. I started that the new bands will be in the same widget but it was not user-friendly, then I tried to create new widgets inside or outside the dock as a new docked widget. But it seemed quite unclear to me.

But I definitely like your approach and I welcome this PR.

I didn't look into the source code, why it can't be compiled to QT5, but hopefully it will work. Thank you.

@kyleboyle
Copy link
Author

kyleboyle commented Feb 8, 2025 •
edited
Loading

Added a checkbox to the band selector now that it is an integral part of the user experience of this feature.
Screenshot 2025-02-07 at 8 30 02 PM

Modified the widget close / delete approach to hopefully work around all of the bugs/gaps I've found in the signalling.

This is ready to test I think. If you are willing to spare some time, please focus on docking the widgets together and closing them, closing the app, making sure the layout state / bands are restored correctly. I can't do much docking testing because it crashes the app in my dev env, even though the official builds work fine.

I spent some time trying to come up with an icon that was better than the plus button, but didn't like anything I came up with that worked on light&dark. I'll stick with the plus sign for now.. If anyone has any suggestions let me know.

@kyleboyle kyleboyle marked this pull request as ready for review February 8, 2025 01:20
@kyleboyle kyleboyle changed the title WIP POC for multiple bandmap windows Multiple independent bandmap windows Feb 8, 2025
@foldynl
Copy link
Owner

foldynl commented Feb 8, 2025

I will test it as soon as possible. in the meantime. @aa5sh @kyleboyle Do you have any idea why MacOS CI Github Action is failing with this?

Info: creating stash file /Users/runner/work/QLog/QLog/build/.qmake.stash Project ERROR: Unknown module(s) in QT: webenginewidgets

It seems that something changes in QT between 6.8.1 and 6.8.2. In Windows this module has also been moved to a separate external module. But I don't know what needs to be installed in Homebrew to make it work.

@kyleboyle
Copy link
Author

@foldynl I'm using the qt version from homebrew which is old 6.7.3

$ brew info qt
==> qt: stable 6.7.3 (bottled), HEAD
Cross-platform application and UI framework
https://www.qt.io/
Installed
/opt/homebrew/Cellar/qt/6.7.3 (15,224 files, 666.6MB) *

Ill try updating my formula to the newest and see what happens.

@kyleboyle
Copy link
Author

kyleboyle commented Feb 8, 2025 •
edited
Loading

I upgraded to 6.8.2 and it compiles and runs in QT Creator 15. I started from a fresh project by deleting all the user generated files in the project directory. However this feature is broken for me now - clicking on the plus button doesn't launch a new bandmap widget. I will continue to investigate.

All i did was a brew install qt and it upgraded everything.

@kyleboyle
Copy link
Author

kyleboyle commented Feb 8, 2025 •
edited
Loading

Ok the codebase is functional now, and my dev env seems to be producing much more stable builds so thanks for forcing me to update qt :D

Could you try using macos-14 instead. There are older posts about this problem and the os version seems to play a part.

@foldynl
Copy link
Owner

foldynl commented Feb 8, 2025

I just used the MacOS15 Image in testing_0.42 GitHub CI and everything is working.

@foldynl
Copy link
Owner

foldynl commented Feb 8, 2025

everything is working.

It means that it is possible to compile the testing_0.42 branch under MacOS...Now I am going to test this PR

@aa5sh
Copy link

aa5sh commented Feb 9, 2025 via email

@aa5sh
Copy link

aa5sh commented Feb 9, 2025

I also had 6.7.2 install and just updated to 6.8.2 in homebrew. two things I noticed quickly:

  • If I undock a bandmap then create a new one I was moving it and docked it in the same window as original and it acted weird. Like he header wasn't there to undock it. I clicked around and it appeared and I was able to undock it. I tried again and crashed qlog.
  • If I don't have a bandmap open by default it's like I send a phantom bandmap window appear then close as qlog as opening.
    I'll spend some more time with it but two quick comments.

But I do like the ease of opening a new bandmap, the context menu to change bands, can click one of the new windows and all seems well.

Michael

@foldynl
Copy link
Owner

foldynl commented Feb 9, 2025

I tested the change. I like its simplicity. I only found these issues:

  1. If I save the layout via the Activity Manager, then add a new Bandmap window but don’t save the layout again, this new Bandmap window is shown during the next start even though his position was not saved. In general, your change doesn’t work well with the Activity Manager because when switching between profiles with different layouts, Bandmap windows remain from the previous profile and do not reflect the saved layout.

  2. The second point is more of a consideration. I don’t have a strong opinion yet on whether it’s good or bad. The original window is the VFO Bandmap. If I have the VFO Bandmap on 20m and then open a new Bandmap for 40m, clicking on the 40m band results in having two windows for 40m. I was wondering whether the VFO window should stay on 20m, but in that case, it wouldn’t match its name—VFO Bandmap.

Point 2 is mostly cosmetic, but point 1 needs to be fixed. I’ll also take a look to see how this could be efficiently handled in the Activity Manager.

@kyleboyle
Copy link
Author 

* If I undock a bandmap then create a new one I was moving it and docked it in the same window as original and it acted weird.  Like he header wasn't there to undock it.  I clicked around and it appeared and I was able to undock it.  I tried again and crashed qlog.

I have these types of issues as well but with all floating windows on macos - think perhaps there are dock window / tools window mac os bugs in general, at least while running with qt creator builds. I don't seem to have the same crashes when running official builds from you.

* If I don't have a bandmap open by default it's like I send a phantom bandmap window appear then close as qlog as opening.
  I'll spend some more time with it but two quick comments.

I've had this happen when crashes occur as the geometry / layout gets out of sync with the saved non vfo bandmap state

@kyleboyle
Copy link
Author 

1. If I save the layout via the Activity Manager, then add a new Bandmap window but don’t save the layout again, this new Bandmap window is shown during the next start even though his position was not saved. In general, your change doesn’t work well with the Activity Manager because when switching between profiles with different layouts, Bandmap windows remain from the previous profile and do not reflect the saved layout.

Lada this shows my unfamiliarity with all of the features :) I thought that the activity profiles were only for setting the qso fields and associated equipment. You mention that you will investigate how to integrate with activity profiles - I'll leave it with you for now and if you want me to look into it please let me know.

2. The second point is more of a consideration. I don’t have a strong opinion yet on whether it’s good or bad. The original window is the VFO Bandmap. If I have the VFO Bandmap on 20m and then open a new Bandmap for 40m, clicking on the 40m band results in having two windows for 40m. I was wondering whether the VFO window should stay on 20m, but in that case, it wouldn’t match its name—VFO Bandmap.

This is also something that I thought about as well. My initial thought was that if the user doesn't want a bandmap to follow the VFO they may close it and rely on multiple non vfo bandmaps set at the bands they are switching between. Though I personally wouldn't want to introduce the complexity in the first version - in the future we could add an option to only sync the vfo bandmap if there is not another bandmap open which is on the new band.

@aa5sh
Copy link

aa5sh commented Feb 9, 2025 via email

@foldynl
Copy link
Owner

foldynl commented Feb 10, 2025

Lada this shows my unfamiliarity with all of the features :) I thought that the activity profiles were only for setting the qso fields and associated equipment. You mention that you will investigate how to integrate with activity profiles - I'll leave it with you for now and if you want me to look into it please let me know

That's not a problem. I'll take a look at it. Unfortunately, it's not as simple as it was in the beginning. The main issue is that the Qt bugs this and this significantly complicate things on the Linux platform.

This is also something that I thought about as well. My initial thought was that if the user doesn't want a bandmap to follow the VFO they may close it and rely on multiple non vfo bandmaps set at the bands they are switching between. Though I personally wouldn't want to introduce the complexity in the first version - in the future we could add an option to only sync the vfo bandmap if there is not another bandmap open which is on the new band.

Yes, I agree with that. It needs to be predictable and simple. And I think it is. It's the only thing that "surprised" me as a user.

Thank you for this PR.

@foldynl
Copy link
Owner

foldynl commented Feb 27, 2025

I tried to reorganize it so that it would be easy to integrate it into Activity Manager. Can @kyleboyle please test it and see if it works on MacOS? I have created a new branch BandmapMultiDockWidget2 for it.

@kyleboyle
Copy link
Author

Works as expected, great job. I can switch activities and the arrangements follow.

@aa5sh
Copy link

aa5sh commented Feb 28, 2025 via email

@foldynl
Copy link
Owner

foldynl commented Feb 28, 2025

Works as expected, great job. I can switch activities and the arrangements follow.

Thanks for the test. There's still work to do but your test confirmed that it's going in the right direction.

@foldynl
Copy link
Owner

foldynl commented Feb 28, 2025

Could we consider changing the readme and wiki so it says MacOS is
supported now and not just for developers? I feel like MacOS version in
general is pretty stable now?

I agree. If you say it's OK, I can change it. I also put it there because I wasn't able to give people advice.

If I have both of you as @aa5sh @kyleboyle, if you agree, I’d like to add you to the Authors list in Help->About. I think it would be good to add you to a place where regular users can see it. If you agree, just send me the string (name/nickname/callsign) I should add. I'm talking about this dialog.
image

Thanks for your work!

@aa5sh
Copy link

aa5sh commented Feb 28, 2025 via email

@foldynl foldynl moved this to In Progress in v0.43.0 Roadmap (closed) Mar 2, 2025
@foldynl foldynl moved this from In Progress to Done - branch: testing_0.43 in v0.43.0 Roadmap (closed) Mar 20, 2025
@kyleboyle
Copy link
Author

Ladislav, I see progress in the 43 branch, great stuff! Please notify if/when you'd like me to do more testing.

Sorry I missed your previous comment asking for call sign - VE9KZ.. thanks so much for including me!

@foldynl
Copy link
Owner

foldynl commented Mar 25, 2025

Please notify if/when you'd like me to do more testing.
@kyleboyle : To be honest, it was more complicated than I thought. I found several "surprise in Qt" and even one bug that Qt has left unfixed for a long time. So, testing is welcome.

The testing branch is dedicated for this pre-release testing. I'm quite worried about this change because some GUI combinations can lead to crashes—for example, when you have detached windows and merge them into one outside the main application window. Then, if you detach one of them again, the application crashes. It is known QT issue, but without fix and without any work-around.

@kyleboyle
Copy link
Author

Are the qt crashes a result of the bandmap windows or do the crashes happen when reconfiguring any of the other widgets as well? Perhaps it is more worrisome with this change because users are more likely to be playing around with the window layouts with all of the bandmap windows?

@foldynl
Copy link
Owner

foldynl commented Mar 25, 2025

It's better to show it. The crash occurs when I try to detach a window. This is running under Qt 6.8.2 on Linux. It's not an issue with the new bandmaps; it's an issue of Qt —I found a bug report about it somewhere. Under Qt 5.x, the behavior is slightly different—the application exits, but the detached window remains. I probably won’t be able to fix this.

I'm still testing the bandmaps because I've already found many issues related to profile switching and the allocation/deallocation of bandmaps.
crash.webm

@aa5sh
Copy link

aa5sh commented Mar 25, 2025 via email

@foldynl
Copy link
Owner

foldynl commented Mar 25, 2025

On Windows, it's more unpredictable. Sometimes it behaves the same as on Linux—crashing. Other times, it correctly performs the detach, but the moved window becomes unresponsive. But as I mentioned, it is a QT bug, so there's probably nothing we can do about it in QLog.

I'm mainly testing in general to make sure the multiple bands windows behave as expected for users. And what does "expected" means - I don’t have an exact definition for it. I must not be surprised by what it does.

@kyleboyle
Copy link
Author

I think Michael and I have griped about this before. I support his claim that running from qt creator seems to increase the likelihood of crashing (for me in macos). I rarely get crashes when running official builds from his repository for macos, and never on windows (I haven't been purposefully testing much on windows, but I use windows when on the radio).

Ladislav: I also have not been able to get deterministic crashing in macos - sometimes the floating window layout changes work, sometimes they crash.

@foldynl
Copy link
Owner

foldynl commented Mar 25, 2025

I'm aiming for a release on April 4th. It's not good to release QLog this week right before the CQ WW WPX SSB – even though I don't think anyone would use it during the contest, but I'll be able to test it with high QSO traffic from DXC.

@aa5sh
Copy link

aa5sh commented Mar 25, 2025 via email

@kyleboyle
Copy link
Author

high QSO traffic on DXC did you notice how something was flooding the networks on Sunday for an hour or so off and on?

The last couple big contests have seen some bad actors flooding the clusters with garbage spots. If you go to dxheat and look at the chat usually people there will be complaining about it if you want to verify if the flooding is happening or not.

@foldynl
Copy link
Owner

foldynl commented Mar 25, 2025

I know this doesn't belong in this thread, but could this be prevented somehow in QLog? We have QSO deduplication. Wasn't that effective?

@aa5sh
Copy link

aa5sh commented Mar 25, 2025 via email

@kyleboyle
Copy link
Author

I know this doesn't belong in this thread, but could this be prevented somehow in QLog? We have QSO deduplication. Wasn't that effective?

From what I remember the spots were technically valid but they were fake, using callsigns that dont exist or weren't active

@foldynl
Copy link
Owner

foldynl commented Apr 4, 2025

added to release 0.43

@foldynl foldynl closed this Apr 4, 2025
@kyleboyle kyleboyle deleted the multi-bandmap branch April 8, 2025 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
v0.43.0 Roadmap (closed)
Status: Done - branch: testing_0.43
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multiple windows of bandmap
3 participants
@kyleboyle @aa5sh @foldynl