-
Notifications
You must be signed in to change notification settings - Fork 104
core: frontend: add notification for uncalibrated sensors #3443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: frontend: add notification for uncalibrated sensors #3443
Conversation
Reviewer's GuideThis PR introduces a real-time notification in the main App when any IMU or compass sensors remain uncalibrated by tracking their status via Vue watchers and storing the flag in the Vuex autopilot module. Sequence diagram for real-time uncalibrated sensor notification updatesequenceDiagram
participant OnboardSensorsVue
participant AutopilotStore
participant AppVue
OnboardSensorsVue->>OnboardSensorsVue: Watch imu_is_calibrated/compass_is_calibrated
OnboardSensorsVue->>AutopilotStore: setUncalibratedSensors(flag)
AppVue->>AutopilotStore: Read uncalibrated_sensors
AppVue-->>User: Show/hide uncalibrated sensors notification
Class diagram for updated AutopilotStore and App.vueclassDiagram
class AutopilotStore {
+bool uncalibrated_sensors
+setUncalibratedSensors(bool uncalibrated)
}
class App {
+bool uncalibrated_sensors()
}
App --> AutopilotStore : uses uncalibrated_sensors
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nicoschmdt - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `core/frontend/src/components/vehiclesetup/overview/OnboardSensors.vue:321` </location>
<code_context>
+ this.updateUncalibratedSensorsStatus()
},
methods: {
+ updateUncalibratedSensorsStatus() {
+ autopilot_data.setUncalibratedSensors(
+ Object.values(this.imu_is_calibrated).some((calibrated) => !calibrated)
+ || Object.values(this.compass_is_calibrated).some((calibrated) => !calibrated),
+ )
+ },
print_bus(bus: BUS_TYPE): string {
</code_context>
<issue_to_address>
Potential issue if imu_is_calibrated or compass_is_calibrated are not objects.
If imu_is_calibrated or compass_is_calibrated can be null or undefined, Object.values will throw. Add a guard or default to an empty object to avoid runtime errors.
</issue_to_address>
### Comment 2
<location> `core/frontend/src/store/autopilot.ts:41` </location>
<code_context>
verhicle_armed = false
+ uncalibrated_sensors = false
+
get parameter() {
</code_context>
<issue_to_address>
Typo in property name 'verhicle_armed' remains unaddressed.
Please update the property name to 'vehicle_armed' for consistency and maintainability.
Suggested implementation:
```typescript
vehicle_armed = false
```
```typescript
setVehicleArmed(armed: boolean): void {
this.vehicle_armed = armed
}
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
core/frontend/src/components/vehiclesetup/overview/OnboardSensors.vue
Outdated
Show resolved
Hide resolved
c9e5ee4
to
06de407
Compare
hmm... I'm not sure whether we want to put the notification in the widgets area, as the other notifications-like things are happening on the opposite side of the top bar. |
I placed it in the widgets area because the armed notification appears in that same position. But if the uncalibrated sensors notification doesn't need to be as explicit as the armed one, maybe we could just use the notification system instead? |
Hmm, right, although they all share semantics, as you noted, they may differ in priority. The notification system is a good idea too (and perhaps we should do both). I'll let other people input on this. Since this is UX, I hope @ES-Alexander can give his view on it? |
As I understand it:
I can think of a couple of potential interface options:
I personally prefer the heartbeat augmentation approach, because calibration seems like part of the autopilot health, and I'm reluctant to have an ever-growing list of icons people need to know about and be able to interpret. That said, the temporary indicators approach still seems viable, and would at least be better than the current lack of indication. I'm unsure which would be easier to implement - they seem pretty similar in complexity 🤷♂️ |
5b0b82c
to
71dbd61
Compare
71dbd61
to
9de8bf1
Compare
fix: #3292
Calibrated:

Uncalibrated:

Docker image for testing: nicoschmdt/blueos-core/notification-for-uncalibrated-sensors
Summary by Sourcery
Add a dashboard notification for uncalibrated sensors and propagate calibration status through the store
New Features: