Repository navigation
Conversation
Replaces the mock sensor data on the Sensors page with real values from the existing MQTT topics: - sensor_infos/json for sensor metadata (name, unit, min/max and critical thresholds) - sensors/<id>/data for the live values (plain text payload, not JSON - a number-as-string for DOUBLE sensors, the string itself for STRING sensors) Sensors are grouped by category (battery/charging, temperatures, mow motor, other) and rendered with gauges (vertical bar for voltage/current, horizontal for temperature, radial for RPM) whose color zones are computed from each sensor's own min/max and critical values instead of hardcoded ones. Each sensor card subscribes only to its own value, so one sensor updating doesn't re-render every other card's gauge. No firmware/ROS changes needed - this only consumes topics that were already being published.
Every threshold in the firmware's Power.cfg (battery/charge critical low/high, empty/full) defaults to -1 when unset, and gets resolved to a real value via launch-file fallbacks - but the C++ code that sets has_critical_low/has_critical_high only checks "value != 0", not whether it's still the raw -1 sentinel. If that fallback doesn't run for some reason, an unresolved -1 leaks through as has_critical_high: true, upper_critical_value: -1 - and since every real sensor reading is >= -1, that sensor would show as permanently critical. computeGaugeScale and isCritical now also require the threshold value itself to be non-negative before treating it as set.
xbot_positioning.cpp hardcodes position_accuracy to exactly 999 when there's no absolute GPS fix (EKF running on dead reckoning only, or GPS timed out). Displayed as "999.00 m" that reads like a real, huge accuracy value instead of what it actually means - no fix at all. IdleBehavior.cpp and DockingBehavior.cpp also turn GPS off entirely around IDLE/DOCKING (back on when undocking/mowing starts) to save power while parked/docked, so those two states are shown as "GPS off" instead of the more general "No fix" - it's expected there, not a signal problem. While actually charging (a safe proxy for "physically in the dock" - IDLE alone can also mean paused out on the lawn), an additional "(Docking Station)" caption makes the reason explicit.
Voltage gauges (V Battery, V Charge) turned out to add no real value: the meaningful range is a tiny sliver of whatever domain gets picked, so most of the bar just sits in one color regardless of the actual reading. rovo89's legacy app has the same min/max fields on its sensor model but never actually uses them for a gauge either. V Battery now leads with the precise battery percentage (already computed server-side, from robot_state/json) and shows the raw voltage as a small secondary line - more useful than a voltage number alone. Both V Battery and V Charge lose the vertical gauge; current sensors keep theirs since amps have a genuinely meaningful zero-to-limit range.
Four separate cards (V Battery, V Charge, Charge Current, plus the Charge State chip riding along on Charge Current) made it hard to answer the one question that actually matters: how's the battery doing. Replaced with a single card: battery percentage as the headline (with a phone-style battery icon that fills/empties and switches to the charging variant), the charge state as a chip, and the three raw readings labeled Battery/Charger/Current underneath instead of three separate unlabeled boxes.
The blade motor only spins while actually mowing - Mow Motor Current and Mow Motor Revolutions are always 0 the rest of the time (docked, idle, undocking, ...). A full vertical/radial gauge just to show an expected zero is visual noise; both cards now say "Motor off" and skip the gauge outside the MOWING state, matching the same treatment already used for GPS off/no-fix.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Same idea as the Battery summary card: showing "Motor off" twice (once for Current, once for RPM) was redundant, since both are always 0 outside of MOWING for the same reason. Now it's a single "Mow Motor" card saying "Motor off" while idle/docked/etc, and the normal two cards with real gauges while actually mowing.
They were landing diagonally opposite in the 2-column grid (arrival order was left, mow, mow-motor, right). Sorting left/right ESC first within Temperatures puts them side by side, which is the more natural pairing to compare at a glance.
…age reading while not charging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the mocked Sensors page with real data from the mower (sensor_infos/json + sensors//data over MQTT).
No firmware changes needed, just consumes topics that are already there.