Skip to content

Frontend: power failsafe/setup updates#3969

Open
Williangalvani wants to merge 4 commits into
bluerobotics:masterfrom
Williangalvani:power_updates
Open

Frontend: power failsafe/setup updates#3969
Williangalvani wants to merge 4 commits into
bluerobotics:masterfrom
Williangalvani:power_updates

Conversation

@Williangalvani

@Williangalvani Williangalvani commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary by Sourcery

Improve visibility and handling of unavailable power configuration parameters and reboot requirements, and add dependency-aware presentation for battery-related failsafes.

New Features:

  • Show a warning alert and reboot action when parameter changes require an autopilot reboot.
  • Indicate when power configuration parameters (battery monitor, sensor pins, multipliers, capacity, arming voltage) are unavailable for the current vehicle.
  • Display failsafe cards as unavailable when their required dependency parameter indicates the feature is disabled, with explanatory messaging.
  • Add dependency metadata for the low-battery failsafe based on the battery monitor setting.

Enhancements:

  • Relax parameter label and editor components to support missing parameters with clearer tooltips and disabled styling.
  • Update inline parameter editor to correctly flag the system as requiring a reboot when changing reboot-required parameters.
  • Unify styling for disabled and unavailable failsafes with clear visual badges and hover behavior.

Williangalvani and others added 4 commits June 29, 2026 12:26
Display parameter labels and a transparent "not found" indicator even when
the parameter is missing, gate the preset selector and editors on parameter
availability, and surface a reboot note with a reboot button since enable
parameters with rebootRequired can hide their dependent parameters until reboot.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Williangalvani Williangalvani marked this pull request as ready for review June 30, 2026 17:13

@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:

  • In FailsafeCard.vue, dependency_unmet manually searches autopilot_data.parameters even though the component already has a params map; consider reusing this.params[dep.paramName] for consistency and to avoid duplicating the lookup logic.
  • Now that ParameterLabel supports missing params and uses different icons/colors, consider adding appropriate aria-label or title attributes to the icon to make the distinction between "info" and "unavailable" accessible to screen readers.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `FailsafeCard.vue`, `dependency_unmet` manually searches `autopilot_data.parameters` even though the component already has a `params` map; consider reusing `this.params[dep.paramName]` for consistency and to avoid duplicating the lookup logic.
- Now that `ParameterLabel` supports missing params and uses different icons/colors, consider adding appropriate `aria-label` or `title` attributes to the icon to make the distinction between "info" and "unavailable" accessible to screen readers.

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 ✏️

Frontend-only UX polish for the failsafe and power-configuration cards: ParameterLabel and BatteryCard now render disabled-but-visible rows when a parameter is missing instead of hiding them, FailsafeCard gains a dependsOn mechanism that surfaces an "UNAVAILABLE" alert (used for the Low-Battery failsafe gated on BATT_MONITOR != 0), BatteryCard shows a reboot-required banner with an inline reboot button, and InlineParameterEditor fixes the inverted setRebootRequired flag when saving a reboot-required parameter.

1. Correctness & Implementation Bugs

  • 1.1 [nit] core/frontend/src/components/vehiclesetup/configuration/failsafes/FailsafeCard.vue:113depParam.value === dep.disabledValue uses strict equality between (potentially) floating-point param values and the typed-as-number disabledValue. For BATT_MONITOR this is fine (integer-enum semantics), but if dependsOn is later reused for a float param, this will silently miss matches. Consider documenting that disabledValue is intended for integer-enum params, or compare with a small epsilon.

5. UI / UX

  • 5.1 [minor] core/frontend/src/components/vehiclesetup/configuration/power/BatteryCard.vue — the new reboot-required banner reads autopilot_data.reboot_required, which is a global flag set by parameter changes anywhere in BlueOS. The banner will therefore appear on the Battery card after editing an unrelated reboot-required parameter elsewhere. The wording ("Some changes require a reboot…") is generic enough to be technically accurate, but a user landing on this page may assume the reboot is needed for a battery change they did not make. Either accept this as intentional or scope the banner to changes initiated from this card.
  • 5.2 [nit] core/frontend/src/components/vehiclesetup/configuration/power/BatteryCard.vue — the Battery Capacity and Min Arming Voltage rows have been moved out of the v-expand-transition block (previously they sat inside it next to the advanced section). The net effect is a behaviour change — those rows are now always visible whenever the monitor is enabled, rather than appearing alongside the advanced section. This is plausibly intentional, but it is not mentioned in the PR description; please confirm.

6. Code Quality & Style

  • 6.1 [nit] core/frontend/src/components/vehiclesetup/configuration/power/BatteryCard.vue<span v-else class="caption text--disabled">Not available</span> is duplicated across seven <v-row> blocks. A tiny <not-available-text/> component (or folding the placeholder into ParameterLabel / a shared wrapper that renders either the editor or the placeholder given a param prop) would remove the repetition and keep future tweaks (wording, styling) in one place.
  • 6.2 [nit] core/frontend/src/components/vehiclesetup/configuration/power/BatteryCard.vue:407 — the new reboot_required computed is a one-liner that just re-exposes autopilot_data.reboot_required. You can drop the computed and reference autopilot_data.reboot_required directly in the template (consistent with how RebootButton.vue reads it), or inline it via a mapState-style getter; the indirection adds no value here.
  • 6.3 [nit] core/frontend/src/components/vehiclesetup/configuration/failsafes/FailsafeCard.vuedependency_message returns '' when there is no dependsOn, but the template only renders the alert under v-else of dependency_unmet, so the fallback is unreachable. Either drop the ?? '' (and assert the dependency exists when shown) or type the computed to reflect that it is only consulted when dependency_unmet.

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

@ES-Alexander

Copy link
Copy Markdown
Collaborator

Relevant issues:

  1. Battery failsafe (still ?) disappears after you enable a battery sensor, before rebooting the autopilot
  2. Long options make the dropdown wider than the edge of the card:
    Screenshot 2026-07-03 at 5 59 51 am

Unrelated issues:

  1. The "Power" setup page should logically come before the failsafes page, since the failsafes only make sense for sensors that are already configured (and calibrated / determined as valid)
  2. The "Lights" setup page can come after the failsafes page if relevant, since it's for independent output configuration
  3. The "Lights" setup page is currently setting and looking for RCIN9/10 values instead of lights1/2
  4. The "Camera Gimbal" page servo reversing toggle appears even though the gimbal is disabled
    Screenshot 2026-07-03 at 6 11 25 am
  5. The disabled gimbal card provides instructions on how to enable a gimbal mount, but it could likely just present a button that enables doing it directly.

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.

2 participants