fix(internet): auto-fill and clamp MTU on connection type switch (#1083)

[PeterJhongLinksys]

What

Fixes the two MTU pain points reported in #1083 when switching WAN
connection type in Internet Settings edit mode:

1. Over-limit value not clamped — switching DHCP/Static → PPPoE kept
   MTU at 1500, which exceeds PPPoE's 1492 limit and forced the user to
   fix it manually.
2. Empty field after leaving Bridge — Bridge mode sets MTU to auto
   (0); switching back to another type left the input blank.

How

• Add a single MTU-range source of truth on UspWanConnectionType
  (mtuMin / mtuMax / clampMtu), replacing the range logic that was
  duplicated in the view.
• updateConnectionType now normalizes MTU for the new type: keep the
  current value when it fits [mtuMin, mtuMax], otherwise fall back to
  the type max. Bridge continues to use auto (mtu = 0).
• The optional-settings view now references the enum getters instead of
  its own switch.

Behavior (matches issue expectations)

• Over-limit values are clamped: 1500 → 1492 on PPPoE, 1500 → 1460 on
  PPTP/L2TP.
• Empty/auto values are auto-filled with the type max when leaving Bridge.
• An already-valid value is preserved across the switch (e.g. 789).

Testing

• Added unit tests for mtuMin / mtuMax / clampMtu and for
  updateConnectionType clamp/auto-fill/preserve behavior.
• test/page/internet_settings/ — all passing; flutter analyze clean.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[AustinChangLinksys]

🤖 Automated Review — Round 1 · 3cbee8b..1eb0882 (full)

Verdict: ✅ APPROVE LGTM — No Critical or Warning findings. Architecture direction is sound; all issues are Suggestions.

⚠️ Note on review methodology: Both sub-agents flagged pptp/l2tp as undefined enum members (Critical). This finding was invalidated by direct API verification of the PR HEAD — the enum at 1eb0882 already contains both members (added in a prior PR). The form validator is also already updated. Sub-agents worked from a behind-HEAD local clone; findings were fact-checked before posting.

---

Suggestions

#	Conf.	File	Issue
S1	High	usp_wan_connection_type.dart:69	mtuMin is a universal constant (RFC 791 IPv4 min = 576); as an instance getter it misleads readers into expecting per-type variance like mtuMax
S2	High	usp_wan_connection_type.dart:85	clampMtu naming implies standard clamp semantics (< min -> min); actual behavior for out-of-range-low is -> mtuMax; docstring covers this but name creates a trap for future callers
S3	Med	usp_internet_settings_form_validator.dart:87-93	Validator adds its own pptp || l2tp => 1460 inline switch instead of delegating to connectionType.mtuMax — minor deduplication gap in the PR's own SSOT goal (values are currently identical, so no correctness risk)
S4	Med	usp_wan_connection_type_test.dart	No explicit test case for clampMtu(0) (the auto-sentinel); behavior tested indirectly via notifier tests but a named model-level test would make the mtu=0 -> auto-fill invariant explicit

---

Suggestion details

S1 — mtuMin should be static const [High confidence] [both reviewers]
lib/page/internet_settings/models/usp_wan_connection_type.dart:69

// Current (instance getter — implies per-type variance):
int get mtuMin => 576;

// Suggested:
static const int mtuMin = 576; // RFC 791 IPv4 minimum, type-independent

This does not change behavior — Dart allows instanceValue.staticMember — but accurately signals to readers that the value does not vary by connection type.

---

S2 — clampMtu semantics vs. naming [High confidence] [both reviewers]
lib/page/internet_settings/models/usp_wan_connection_type.dart:85

// Current — "clamping" a below-min value returns mtuMax, not mtuMin:
int clampMtu(int mtu) => (mtu >= mtuMin && mtu <= mtuMax) ? mtu : mtuMax;

The docstring explicitly documents this ("falls back to [mtuMax]"), so there is no correctness risk today. The naming concern is that Dart num.clamp(lower, upper) always returns lower for below-min inputs, so future callers who rely on method-name semantics without reading the docstring may be surprised.

Options:

• Rename to normalizeMtu / autoFillOrClampMtu to signal the non-standard behavior, or
• Keep name and ensure tests explicitly document the below-min behavior

---

S3 — Validator inline vs. SSOT delegation [Med confidence]
lib/page/internet_settings/providers/usp_internet_settings_form_validator.dart:87

This PR correctly establishes connectionType.mtuMax as SSOT for the view and notifier paths, but the form validator still carries its own inline switch with hardcoded 1460. For the current types the values match exactly (no correctness risk). If a sixth protocol type is added in the future, the validator and enum would need to be updated in sync. Consider:

// Instead of inline switch:
final mtuMax = form.connectionType.mtuMax;

---

S4 — Named test for mtu=0 (auto-sentinel) [Med confidence]
test/page/internet_settings/models/usp_wan_connection_type_test.dart

The codebase uses mtu=0 as the "auto/unset" sentinel. The fact that clampMtu(0) -> mtuMax is what triggers auto-fill on type switch is the core behavior of this PR. Consider adding:

test('"auto" sentinel (mtu=0) auto-fills to mtuMax on type switch', () {
  expect(UspWanConnectionType.dhcp.clampMtu(0), 1500);
  expect(UspWanConnectionType.pppoe.clampMtu(0), 1492);
  expect(UspWanConnectionType.pptp.clampMtu(0), 1460);
  expect(UspWanConnectionType.l2tp.clampMtu(0), 1460);
});

What looks good

• Single source of truth refactor is complete in the view: _mtuMin/_mtuMax in usp_optional_section.dart now delegate to connectionType.mtuMin/mtuMax — the root cause of #1083 is addressed
• pptp/l2tp correctly included in mtuMax with accurate overhead values (PPPoE 8-byte PPP header -> 1492; PPTP/L2TP 40-byte tunnel overhead -> 1460)
• isPppBased is actively used in usp_internet_settings_service.dart (lines 208, 214, 223, 259, 261) — not dead code
• Bridge path correctly preserved: mtu=0 sentinel assignment guarded by if (type == bridge) before the clamp branch
• clampMtu is pure and side-effect-free — safe to call in switch/computed properties
• Test coverage is solid: 119 new lines across model tests (mtuMin/mtuMax/clampMtu per type) and three notifier behavioral scenarios (PPPoE clamp, bridge auto-fill, in-range preserved)
• No security concerns: mtu is type int throughout, never interpolated into raw USP queries; no hardcoded credentials introduced

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Critical findings from sub-agents invalidated by direct PR HEAD verification before posting. Automated — please sanity-check before merge.