frontend: autopilot: trigger frontend to re download autopilot parameters #3623
Conversation
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a reset timestamp and message debounce in the ParameterFetcher to discard stale MAVLink parameters after a reset, refactors the ParametersTable to key entries by parameter name for direct updates, and triggers an autopilot store reset on board changes to force parameter re-download. Sequence diagram for autopilot board change triggering parameter re-downloadsequenceDiagram
actor User
participant "BoardChangeDialog.vue"
participant "autopilot_data (store)"
participant "ParameterFetcher"
User->>"BoardChangeDialog.vue": Initiate board change
"BoardChangeDialog.vue"->>"autopilot_data (store)": reset()
"autopilot_data (store)"->>"ParameterFetcher": reset()
"ParameterFetcher"->>"ParameterFetcher": reset_timestamp = performance.now()
"ParameterFetcher"->>"ParameterFetcher": Clear parameter table
Note over "ParameterFetcher": New parameters will be fetched
Entity relationship diagram for ParametersTable refactorerDiagram
PARAMETERS_TABLE {
string parametersDict
boolean metadata_loaded
}
PARAMETER {
string name
number value
string id
Range range
number default
}
PARAMETERS_TABLE ||--o{ PARAMETER : contains by name
Class diagram for updated ParameterFetcher and ParametersTableclassDiagram
class ParameterFetcher {
- min_update_interval_ms: number
- reset_timestamp: number
+ constructor()
+ reset()
+ updateStore(): void
+ setupWs(): void
}
class ParametersTable {
- parametersDict: Parameter[]
- metadata_loaded: boolean
+ updateParam(param_name: string, param_value: number): void
+ parameters(): Parameter[]
}
ParameterFetcher --> ParametersTable: uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Extract the 500ms “ignore old messages” window into a named constant or configuration option instead of a magic number for easier tuning and clarity.
- Rename reset_timestamp to a more descriptive identifier (for example ignoreWindowStart) so its purpose of suppressing stale MAVLink messages is clearer.
- Since parametersDict now uses string keys, consider introducing a type alias or renaming it (e.g. parametersByName) and audit all its consumers to prevent any downstream mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the 500ms “ignore old messages” window into a named constant or configuration option instead of a magic number for easier tuning and clarity.
- Rename reset_timestamp to a more descriptive identifier (for example ignoreWindowStart) so its purpose of suppressing stale MAVLink messages is clearer.
- Since parametersDict now uses string keys, consider introducing a type alias or renaming it (e.g. parametersByName) and audit all its consumers to prevent any downstream mismatches.
## Individual Comments
### Comment 1
<location> `core/frontend/src/types/autopilot/parameter-fetcher.ts:95-97` </location>
<code_context>
setupWs(): void {
this.listener.setCallback((receivedMessage) => {
+ // Ignore messages received after reset for 500ms
+ const message_age = performance.now() - this.reset_timestamp
+ if (this.reset_timestamp > 0) {
+ if (message_age < 500) {
+ return
</code_context>
<issue_to_address>
**suggestion:** The 500ms message ignore window may be too short or too long depending on system latency.
Consider making the 500ms window configurable to accommodate different latency conditions, or add documentation explaining the choice of this value.
Suggested implementation:
```typescript
this.total_params_count = null
this.watchdog_last_count = 0
this.parameter_table.reset()
this.reset_timestamp = performance.now()
// Configurable window (ms) to ignore messages after reset, to accommodate system latency
this.resetIgnoreWindowMs = 500
}
```
```typescript
// Ignore messages received after reset for configurable window (default: 500ms)
// This helps avoid processing stale messages after a reset, and can be tuned for system latency.
const message_age = performance.now() - this.reset_timestamp
if (this.reset_timestamp > 0) {
if (message_age < this.resetIgnoreWindowMs) {
return
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
nicoschmdt
force-pushed
the
trigger-frontend-to-re-download-parameters
branch
from
080b435 to
74a3d6d
Compare
patrickelectric
approved these changes
Nov 4, 2025
Member
|
Should we backport it? The issue was from Jan 2024 |
Member
yes |
joaoantoniocardoso
added
the
move-to-stable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
fix: #2343
Had to add a timeout to ignore old mavlink messages and avoid having the store set up dated information regarding the autopilot firmware.
Tested going between the SITL board and the Navigator. The
Change BoardandInstall Firmwarecode flow were both considered.Summary by Sourcery
Add a timeout to filter out outdated MAVLink parameter messages after resets, refactor parameter storage indexing to use parameter names, and ensure the frontend store is reset when changing boards to trigger fresh parameter downloads.
Bug Fixes:
Enhancements: