Skip to content

Frontend: allow using outdated param sets#3949

Open
Williangalvani wants to merge 1 commit into
bluerobotics:masterfrom
Williangalvani:outdated_params
Open

Frontend: allow using outdated param sets#3949
Williangalvani wants to merge 1 commit into
bluerobotics:masterfrom
Williangalvani:outdated_params

Conversation

@Williangalvani

@Williangalvani Williangalvani commented Jun 12, 2026

Copy link
Copy Markdown
Member
Screenshot 2026-06-12 at 14 15 39 Screenshot 2026-06-12 at 14 22 37

Summary by Sourcery

Update vehicle parameter set selection to support older-but-compatible firmware versions and clearly distinguish outdated presets in the UI.

New Features:

  • Allow using parameter sets built for older firmware versions when no exact version match exists.

Enhancements:

  • Revamp parameter set buttons to show firmware version labels, highlight outdated sets with warning styling and tooltips, and order current sets before older ones.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The escapeRegex helper is generic and might be useful elsewhere; consider moving it to a shared utility module instead of keeping it local to this component.
  • The candidates structure inside filtered_param_sets is typed inline; extracting this to a named interface/type would make the version-selection logic easier to understand and maintain.
  • The label for each button (item.name.split('/').pop()?.replace(/\.params$/i, '')) is derived on each render in the template; consider precomputing a display_label field in filtered_param_sets to avoid repeating this string manipulation in the view.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `escapeRegex` helper is generic and might be useful elsewhere; consider moving it to a shared utility module instead of keeping it local to this component.
- The `candidates` structure inside `filtered_param_sets` is typed inline; extracting this to a named interface/type would make the version-selection logic easier to understand and maintain.
- The label for each button (`item.name.split('/').pop()?.replace(/\.params$/i, '')`) is derived on each render in the template; consider precomputing a `display_label` field in `filtered_param_sets` to avoid repeating this string manipulation in the view.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@github-actions

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: MINOR SUGGESTIONS ✏️

Reworks filtered_param_sets in ParamSets.vue so that, when no exact-version paramset exists, older-but-compatible ones (≤ installed firmware) are still offered. Outdated entries are rendered as warning-colored outlined buttons with an mdi-alert icon and a tooltip explaining the version mismatch; current sets sort first, then older ones newest-first. Pure frontend change, no new deps.

5. UI / UX

  • 5.1 [minor] core/frontend/src/components/vehiclesetup/overview/ParamSets.vue template — the new <span class="version-tag ml-2">v{{ item.version_label }}</span> references a version-tag class that has no rule in the scoped <style> block (and is not defined anywhere else in the frontend). The class is effectively dead; either drop it or add the intended treatment (e.g. smaller font, muted color, pill background) so the version label is visually distinct from the button text.
  • 5.2 [minor] With the new logic, every paramset whose version is ≤ current is shown, not just the closest match. For vehicles with many historical paramsets (e.g. 4.0, 4.1, 4.2, 4.3, 4.4, 4.5, 4.5.3) this can produce a long row of warning buttons next to the current one. flex-wrap keeps the layout intact, but consider capping the number of outdated suggestions (e.g. the N newest) or collapsing them behind a "Show older sets" expander to avoid overwhelming the user.
  • 5.3 [nit] When best === 2 (an exact-patch paramset exists) and a same-major.minor paramset without a patch is also present, the patch-less one is flagged outdated and its tooltip reads e.g. "Outdated: built for firmware 4.5 (current is 4.5)" — the same version twice. Including the patch in the "current is …" portion (e.g. ${version.major}.${version.minor}.${version.patch}) would make the distinction explicit.

6. Code Quality & Style

  • 6.1 [minor] ParamSets.vue computed filtered_param_sets declares an inline candidate type { name: string, paramset: Dictionary<number>, version: SemVer, version_label: string, has_patch: boolean }[] that duplicates the FilteredParamSet interface defined just above the component. Extending FilteredParamSet with has_patch (or defining a small Candidate alias) would remove the duplication and the typeof candidates[number] indirection used in specificity.
  • 6.2 [minor] specificity(c) is invoked twice per candidate — once inside the reduce that computes best, then again in the map that sets outdated. Precompute the score per candidate (const scored = candidates.map(c => ({ c, score: specificity(c) }))) so each candidate is evaluated once and the outdated decision becomes a single comparison against best.
  • 6.3 [nit] The fallback display name uses item.name.split('/').pop()?.replace(/\.params$/i, ''). String.prototype.split always returns a non-empty array, so pop() is statically a string here — the ?. is unnecessary and slightly muddies the inferred type.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

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.

1 participant