Skip to content

SettingsValidator.ValidRanges is stale/wrong for several fields — GoProSettingsData.defaultSettings() fails its own validation #94

Description

@kmatzen

Problem

SettingsValidator.ValidRanges (Facett/SettingsValidator.swift) checks several settings against ClosedRanges that don't match the real GoPro protocol value domains used elsewhere in the app (CameraSettingDescriptions.swift, ConfigurationUtils.swift, GoProCommands.swift). As a result, ConfigValidation.validateConfig currently reports the app's own GoProSettingsData.defaultSettings() as invalid — caught while adding unit test coverage in #93 (ConfigValidationTests.testValidateConfigWithDefaultSettingsIsValid had to be watered down to avoid asserting a false invariant).

Already fixed in #93: mode (was 0...1, real values go up to 27) and lcdBrightness (was 0...2, it's actually a 0-100 percentage).

Still broken, found by comparing ValidRanges against CameraSettingDescriptions.swift's switch statements and GoProSettingsData.defaultSettings()'s actual defaults:

Field ValidRanges Real default value Real valid values (from CameraSettingDescriptions)
antiFlicker 0...1 2 (60Hz) {2, 3} (60Hz, 50Hz)
rawAudio 0...1 2 (High) {0, 1, 2, 3}
hindsight 0...1 4 (Off) {2, 3, 4}
whiteBalance 0...1 4 (Native) {0, 2, 3, 4, 5, 7, 8, 9, 10, 11, 12}
wind 0...1 0 (Off, OK) {0, 2, 4}1 is currently accepted but invalid
led 0...2 5 (Off) needs checking against ledDescription

Note that several of these (whiteBalance, wind, hindsight, rawAudio) are sparse, non-contiguous ID sets, not dense ranges — GoPro's protocol reuses small integers as opaque IDs, not sequential enum indices. A ClosedRange (a...b) can't correctly represent "valid IDs are {0,2,3,4,5,7,8,9,10,11,12}" — it will always either reject real values or accept invalid gaps (e.g. 1, 6 for whiteBalance). Fixing this properly likely means switching from ClosedRange bounds-checking to explicit Set<Int> membership checks per field, ideally derived from (or shared with) the switch statements already in CameraSettingDescriptions.swift so the two can't drift apart again.

Suggested approach

  1. Audit every field in ValidRanges against its corresponding CameraSettingDescriptions switch statement (and GoProCommands.swift where descriptions don't exist) to enumerate the real valid ID set.
  2. Replace ClosedRange checks with Set<Int> (or similar) membership checks for sparse fields; keep ranges only for fields that are genuinely dense/contiguous (e.g. videoResolution, isoMin).
  3. Add a test asserting SettingsValidator.shared.validateSettings(GoProSettingsData.defaultSettings()) (and ideally all 5 DefaultConfigurations.createDefaultConfigs() presets) don't throw — this is the regression guard that should have existed already.

Impact

Any UI path that calls ConfigValidation.validateConfig/validateCurrentConfig (e.g. before applying/saving a config) may currently show spurious validation errors for real, working camera settings.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions