Skip to content

SliceRecreatePolicy: restoredPanCenterMhz <= 0 fallback is unreachable in production #3416

@ten9876

Description

@ten9876

Summary

Identified by AetherClaude during review of #3316 (merged), confirmed non-blocking by @rfoust. Filing as a small follow-up.

SliceRecreatePolicy::decide() has a fallback path for "the radio has not reported a pan center yet" gated on restoredPanCenterMhz <= 0. The fallback is dead code in production:

PanadapterModel::m_centerMhz is default-initialized to 14.1, not 0. So when the pan has been claimed but applyPanStatus(center=…) hasn't run yet, restored->centerMhz() returns 14.1 — the policy's > 0 check sees a positive value and skips the fallback entirely.

The two tests covering that branch — testRestoredPanCenterNotYetKnownFallsBackToLastFreq and testRestoredPanCenterUnknownNoSettingsFallsBackToDefault — feed restoredPanCenterMhz = 0.0, a state the production code can't actually reach. The comment in SliceRecreatePolicy.h ("<= 0 means the radio has not reported a center yet (pan only just claimed)") and the matching tests overstate the guarantee.

In practice this is harmless — in the #3212 trace the pan center was populated 1.8s before slice-list returned, and even if it weren't, 14.1 MHz is inside 20m which is a common-enough restore target — but it's a real precision gap between the docs/tests and the production state machine.

Two options, either is fine

Option 1 — make the fallback actually live (more defensive)

Add a bool m_centerReported{false} flag to PanadapterModel, flip it to true in applyPanStatus whenever kvs.contains("center"), expose bool centerReported() const. Have SliceRecreatePolicy::Inputs gate on restoredPanCenterReported instead of restoredPanCenterMhz > 0. Production code can then actually hit the fallback when a pan is claimed but the radio hasn't sent its first display pan ... center= yet.

Option 2 — tighten the comments + drop the unreachable tests

Update SliceRecreatePolicy.h to reflect that pan-center is essentially always used on the reuse path (the <= 0 branch only catches a theoretical zero-init that PanadapterModel doesn't produce). Drop the two unreachable tests. Minimal-change option.

Context

Tagging @jensenpat as PR author in case he wants to pick this up — it's a 10-30-minute cleanup PR depending on which option you choose. Happy for anyone else to grab it too.

Principle IX (Surface Only What Survives).

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinggood first issueGood for newcomersmaintainer-reviewRequires maintainer review before any action is takenmulti-panMulti-panadapter layout and slicingpriority: lowLow priority

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions