Skip to content

core: frontend: StatusTextWatcher: Color status text - #4156

Open
patrickelectric wants to merge 1 commit into
bluerobotics:masterfrom
patrickelectric:color-motor-status
Open

core: frontend: StatusTextWatcher: Color status text#4156
patrickelectric wants to merge 1 commit into
bluerobotics:masterfrom
patrickelectric:color-motor-status

Conversation

@patrickelectric

Copy link
Copy Markdown
Member

The motor detection status lines all look the same, even on failure.

Fix #2801

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
@github-actions

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: MINOR SUGGESTIONS ✏️

Replaces the disabled v-textarea in StatusTextWatcher with a v-fored <div> list and colors each line by scanning the text for keywords like failed, bad thrust, is ok, reversed, complete. Targets the motor-detection status flow (Fix #2801).

1. Correctness & Implementation Bugs

  • 1.1 [minor] core/frontend/src/components/common/StatusTextWatcher.vue:71colorFrom matches on the raw substring complete, so any status text containing incomplete (e.g. Calibration incomplete, PreArm: init incomplete) will render as green success--text. Same shape of risk for reversed matching not reversed. Consider word-boundary regexes (/\bcomplete\b/) or, better, drive the color from the MAVLink severity.type field (see core/frontend/src/utils/mavlink_prettifier.ts:63 — the callback already receives the full message, so receivedMessage.message.severity.type is available). Severity-based coloring would also generalize beyond motor detection, since StatusTextWatcher is reused by the compass calibrators.

5. UI / UX

  • 5.1 [minor] core/frontend/src/components/common/StatusTextWatcher.vue:6 — the previous v-textarea auto-grow readonly disabled gave the block a bounded, visually-distinct "log field" affordance. The new bare <div> wrapper has no border, background, or max-height; on long calibration runs the list grows without bound and pushes the surrounding layout. Consider wrapping in a v-card/v-sheet (or keeping a scrollable container with a max-height) so the panel stays contained. Color contrast should also be sanity-checked against the light/dark Vuetify themes — warning--text on the light theme is a low-contrast amber.
  • 5.2 [nit] core/frontend/src/components/common/StatusTextWatcher.vue:10:key="index" is acceptable here because the list is append-only, but a stable key (e.g. an incrementing counter set when a message is pushed) is cheaper for Vue's diff and is the codebase convention elsewhere.

6. Code Quality & Style

  • 6.1 [minor] core/frontend/src/components/common/StatusTextWatcher.vue:65-77 — the keyword list inside colorFrom is effectively an untyped magic table. Lifting it into a module-level const SEVERITY_KEYWORDS: Record<string, string[]> (or the severity-based approach from 1.1) would make it easier to extend and reason about ordering (right now, failed must be checked before complete to avoid mis-coloring "complete failure" — that ordering constraint is implicit).
  • 6.2 [nit] core/frontend/src/components/common/StatusTextWatcher.vue:79 — the deleted blank line before </script> was consistent with the rest of the frontend; not a blocker, but keeping it matches surrounding files.

8. Documentation

  • 8.1 [nit] core/frontend/src/components/common/StatusTextWatcher.vue:65 — a one-line comment (or a JSDoc-free named constant) explaining that these substrings are the motor-detection vocabulary from ArduPilot would help future readers. AGENTS.md forbids parrot comments, but the why (which subsystem's status vocabulary this reflects) is currently invisible.

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.

core: frontend: vehicle-setup: motor reversal detection clarity suggestion

1 participant