Skip to content

Remove dead _previous_frequency state from BigFrequency widget - #3294

Merged
gullradriel merged 3 commits into
nextfrom
copilot/fix-code-review-suggestion
Aug 19, 2026
Merged

Remove dead _previous_frequency state from BigFrequency widget#3294
gullradriel merged 3 commits into
nextfrom
copilot/fix-code-review-suggestion

Conversation

Copilot AI commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

BigFrequency retained a _previous_frequency member that was no longer used in paint(), leaving dead state in the class and making the header disagree with the implementation.

Brief description of what you did

  • Removed rf::Frequency _previous_frequency{~0LL} from BigFrequency in ui_widget.hpp — it was dead state after the paint guard was removed.
  • Removed the if (_frequency != _previous_frequency) guard from paint()paint() is now unconditional, which is correct since the widget framework gates it.
  • Moved the dirty-check to set() to preserve the optimization (skip repaints when frequency hasn't changed) without needing the extra member:
void BigFrequency::set(const rf::Frequency frequency) {
    if (_frequency != frequency) {
        _frequency = frequency;
        set_dirty();
    }
}

This keeps the redraw behavior correct while eliminating the stale member.


Proof that your changes work

Code-only cleanup; no behavioral change to observable output.

🖥️ Proof it compiles

N/A — trusted contributor

📱 Proof of testing on a real device

N/A — no functional change; only removes dead state and restructures the dirty-check.

📡 Proof against a real emitter/receiver (if applicable)

N/A — no RF changes.


📚 Wiki documentation commitment

  • I will (or already) create(d) wiki document for my newly added feature

N/A — internal widget refactor, no user-visible feature change.


Checklist

  • Kept changes minimal and limited to necessary files
  • Verified functionality remains intact and code compiles
  • Attached proof that the code compiles successfully (or marked N/A as a trusted contributor)
  • Attached proof of testing on real PortaPack hardware (or marked N/A as a trusted contributor)
  • Attached proof of testing against a real emitter/receiver (if RF-related), or marked N/A with justification
  • I understand that by getting this PR merged, I am implicitly agreeing to create or update the corresponding wiki page (including a main-screen screenshot, description, controls, and limitations)
  • I own all rights to this code (i.e., all code contained in this PR), including compliant usage rights for third-party libraries, and I agree that this code is licensed under the license of this project (GPL-3.0).
  • If any third-party libraries are used, I confirm that their licenses comply with the requirements for contributing to this repository.
  • Reviewed the Contributing Guidelines

Copilot AI and others added 2 commits August 18, 2026 18:21
… paint() guard

Co-authored-by: gullradriel <3157857+gullradriel@users.noreply.github.com>
Co-authored-by: gullradriel <3157857+gullradriel@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code based on review comment Remove dead _previous_frequency state from BigFrequency widget Aug 18, 2026
Copilot AI requested a review from gullradriel August 18, 2026 18:23
Comment thread firmware/common/ui_widget.cpp
Comment thread firmware/common/ui_widget.cpp

@gullradriel gullradriel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me

@gullradriel
gullradriel marked this pull request as ready for review August 18, 2026 18:25
@gullradriel
gullradriel enabled auto-merge (squash) August 18, 2026 18:26

@zxkmm zxkmm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok

@gullradriel
gullradriel merged commit ea40e3a into next Aug 19, 2026
3 checks passed
@gullradriel
gullradriel deleted the copilot/fix-code-review-suggestion branch August 19, 2026 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants