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.

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

Add feature to update existing bookmark #1306

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

0penBrain
Copy link

This PR mainly adds a feature that an existing can be updated using contextual menu:
image

When updating, frequency + bandwidth + modulation are updated.
Bookmarks are re-sorted and selected bookmark is kept selected at its new location.

Also this PR changes that "Delete bookmark" (and "Update bookmark" as well) is only shown if contextual menu is called when hovering a bookmark. It's not more displayed when right-clicking on an empty area of the QTableView. Only "Add new bookmark" is available in this case.

@@ -84,6 +84,15 @@ struct BookmarkInfo
{
return frequency < other.frequency;
}

bool operator==(const BookmarkInfo &other) const
Copy link
Contributor

@willcode willcode Oct 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this used? In stable_sort or indexOff?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you edited. 😄 This is needed for indexOf.

@argilo
Copy link
Member

argilo commented Oct 7, 2023

I don't understand why this is needed. It is already possible to update a bookmark by clicking on the frequency, name, modulation, or bandwidth.

@0penBrain
Copy link
Author

I don't understand why this is needed. It is already possible to update a bookmark by clicking on the frequency, name, modulation, or bandwidth.

Actually it's possible to edit existing bookmarks, i.e. hand type new values in the fields.
Here the concept of "update" is that frequency+bandwidth+modulation are updated with the current GUI settings. 😉

@0penBrain
Copy link
Author

I have checked that contextual QMenu is correctly deleted by Qt when hidden after being popped up.
Now consistent with BookmarksTagList.
Pushed as a second commit to ease review but can squash on demand.

@argilo
Copy link
Member

argilo commented Oct 7, 2023

Here the concept of "update" is that frequency+bandwidth+modulation are updated with the current GUI settings.

Ah, OK. What do you typically use this feature for?

@0penBrain
Copy link
Author

0penBrain commented Oct 7, 2023

Ah, OK. What do you typically use this feature for?

When exploring a frequency range with sparse emissions. I can display the range at full scale and place coarse bookmarks where the waterfall is showing emissions. Then I can individually zoom in the frequency band and fine-tune each bookmark to more precise freq and bandwidth (and eventually fix the modulation if it was incorrectly set).

@argilo
Copy link
Member

argilo commented Oct 7, 2023

Thanks for the explanation. It does sound useful to be able to update a bookmark that way.

@vladisslav2011
Copy link
Contributor

I have implemented something similar to this functionality in my fork: trying to create a bookmark on top of existing bookmark updates demodulator, filter bandwidth and other settings (most of demodulator/audio processing settings can be stored to a bookmark) instead of creating a new one.
Ah, It's part of this PR #1106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants