feat(dronecan): DroneCAN node configurator tab — GetNodeInfo, GetSet, ExecuteOpcode, RestartNode#2671
Draft
daijoubu wants to merge 38 commits into
Draft
Conversation
- Add MSP2_INAV_DRONECAN_NODES (0x2042) and MSP2_INAV_DRONECAN_NODE_INFO (0x2043) to MSPCodes.js - Parse both responses in MSPHelper.js; add FC.DRONECAN_NODES and FC.DRONECAN_NODE_INFO state slots in fc.js - Add tabs/dronecan.html and tabs/dronecan.js with node table, health badges, 2s auto-refresh, and per-node detail on row click - Register dronecan tab in gui.js allowedTabs and configurator_main.js - Add i18n strings for tab and column headers - Fix nav-toggle-all link being caught by tab-switch handler (showed spurious firmware upgrade warning) CSS health badge styling still pending.
- Fix MSPHelper.js: use 'data' not 'dataView' in DRONECAN_NODES/NODE_INFO parse handlers - Fix dronecan.js: set GUI.active_tab = this (not string) so cleanup() is callable on tab switch - Fix dronecan.js: interval.remove() not interval.kill() (correct API) - Fix dronecan.js: call GUI.content_ready(callback) to remove loading overlay; bare callback() left content permanently hidden - Add case 'dronecan' to tab switch in configurator_main.js - Add CSS health badge colour coding (src/css/tabs/dronecan.css) - Remove commented-out duplicate accordion block from configurator_main.js
Replace bespoke tab-pane/tab-header/tab-content markup with the standard tab-[name]/content_wrapper/tab_title/gui_box pattern used by all other tabs.
…ests Query MSP2_INAV_DRONECAN_NODE_INFO once per new nodeID and cache the name. Subsequent refreshes use the cache with no extra MSP requests.
… table - Show last_seen in node detail panel (now meaningful after firmware fix) - Remove uptime column from node table (not in NODES response; available in node detail via NODE_INFO)
…nvention Use green/amber/red scheme matching ArduPilot Mission Planner and QGroundControl: OK=green, WARNING=amber, ERROR=red, CRITICAL=red. Removes purple CRITICAL which had no precedent in DroneCAN tooling. Also removes duplicate accordion/disable_3d_acceleration handlers introduced by fdd4fcb that caused double-firing on nav group clicks.
Adds CAN baud rate and FC node ID controls to the DroneCAN tab using the generic settings API (MSPV2_SETTING / MSPV2_SET_SETTING). Save & Reboot button follows the standard content_toolbar pattern. Settings are loaded sequentially on tab init to avoid MSPV2_SETTING response race condition.
Three nodes on a bus line, grey/white variants for inactive/active states.
Remove stray NNER property and duplicate ANALOG/ARMING_CONFIG/FC_CONFIG/MISC declarations introduced by a bad hunk placement in the DroneCAN WIP commit.
… cleanup Add DRONECAN_NODES and DRONECAN_NODE_INFO to FC.resetState() so stale node data from a previous connection is cleared before reconnecting. Clear nameCache in dronecanTab.cleanup() so node names from a previous session are not shown when reconnecting to a different FC that reuses the same node IDs.
…NavFlight#7 iNavFlight#8 iNavFlight#10 - MSPHelper: add bounds check before getUint32 in DRONECAN_NODES parser to prevent RangeError on truncated packets (iNavFlight#3) - dronecan.js: replace parallel NODE_INFO requests with sequential fetch to avoid MSP dedup collapsing multiple node lookups to one (iNavFlight#4) - dronecan.js: guard NODE_INFO callback on info.nodeID === nodeId to prevent stale slot from a racing showDetail/render request (iNavFlight#8) - dronecan.js: use textContent for node name in both render and showDetail instead of unescaped innerHTML interpolation (iNavFlight#7) - dronecan.js: fill MODE_LABELS[4-6] with UNKNOWN_N and fall back to MODE_N for out-of-range values instead of silently showing OPERATIONAL (iNavFlight#10)
Fix getGpsProtocols() which was missing CRSF (firmware index 2) and had FAKE at the wrong index. Full list now matches gpsProvider_e enum: 0=UBLOX, 1=MSP, 2=CRSF, 3=FAKE, 4=DRONECAN. When DroneCAN is selected, hide the serial port and baud dropdowns (not applicable for CAN bus) and reset port to NONE so saving does not incorrectly assign a serial port to GPS. Show an info note directing the user to the DroneCAN tab for bus configuration.
Protocol now sits at the top of the GPS config section so port and baud collapsing when DroneCAN is selected doesn't shift it.
Blue background and border made the GPS enable toggle appear to turn blue when port/baud fields collapsed. Match preset_info box style instead.
Applying background to the outer div coloured sibling elements that fell within its visual bounds. Target the span so only the text gets the grey background.
Background box styling visually enclosed adjacent elements. Plain italic text at 0.9em conveys the note without affecting surrounding layout.
Note now sits within the protocol row above its divider, visually grouped with the dropdown that triggered it rather than floating between protocol and SBAS as a separate element.
Documents the Phase 5 wire format and verifies the parser correctly handles 46-byte (base) and 71-byte (with version data) payloads. Currently fails on version fields — passes after Phase 6 extends the MSPHelper.js decoder to read bytes 46-70.
…tail panel MSPHelper.js: extend MSP2_INAV_DRONECAN_NODE_INFO decoder to parse bytes 46-70 (sw_major, sw_minor, sw_optional_field_flags, sw_vcs_commit, hw_major, hw_minor, hw_unique_id) when payload is >= 71 bytes. dronecan.js: add SW Version, HW Version, and Unique ID rows to showDetail() panel, conditionally rendered when version data is present.
Replace synchronous NODE_INFO fetch with dronecanAsyncPoll() helper that works with the new firmware ASYNC_REQUEST/ASYNC_RESULT MSP commands. Seq generation counter guards against stale results across MSP ports. Changes: - MSPCodes: replace NODE_INFO (0x2043) with ASYNC_REQUEST/ASYNC_RESULT - MSPHelper: add ASYNC_REQUEST and ASYNC_RESULT decoders; ASYNC_RESULT handles variable-length GetNodeInfo and GetSet responses including BigInt for int64 values outside Number.MAX_SAFE_INTEGER - fc.js: replace DRONECAN_NODE_INFO with DRONECAN_ASYNC_REQUEST/RESULT - dronecan.js: add dronecanAsyncPoll() helper; update node name fetch and detail panel to use async pattern; add parameter list/edit UI - main.css: add .tab-dronecan to positioned tab list (fixes save button) - dronecan.css: style params table, inputs, and write buttons - Replace defunct NODE_INFO decoder test with ASYNC_RESULT decoder test covering all value types, BigInt edge case, and seq preservation
…e node handling - Add Save to EEPROM button (ExecuteOpcode service 10) and Restart Node button (RestartNode service 5) to the parameter panel - Decode ExecuteOpcode and RestartNode responses in MSPHelper (simple ok-only result, no name prefix) - Wire both services through dronecanAsyncPoll with correct payloads - Live-update Health, Mode, Last Seen, Uptime, Vendor Status in the detail panel on every 2s refresh without re-fetching GetNodeInfo - Close detail panel automatically when the displayed node disappears from the node list (e.g. after a node ID change and restart)
- Add stale callback guard in showDetail (discards response if node changed) - Add cancellation guard in fetchParam chain (same mechanism) - Scope .param-actions selector to #dronecan-params to avoid leakage - Fix font-size 12px → 1em on param buttons (scales with display density) - Fix service_id comment order in MSPHelper (5=RESTART_NODE, 10=EXECUTE_OPCODE) - Add poll window comment (34 × 75 ms ≈ 2.5 s)
…nt, health colour - Add esc() helper; escape all wire-sourced strings (node name, param name, param value) before innerHTML interpolation - Add comment in MSPHelper noting offset is not advanced past value in the PARAM_GETSET switch — harmless today but must be revisited if firmware extends the response with min/max fields - Distinguish health-critical from health-error with dark red (#7b0000) - Add comment on nameCache clear explaining why (stale names across reconnects)
…tants - Add try-catch around MSPHelper READY decoder to handle truncated responses - Copy hw_unique_id with .slice() to avoid holding a live view into MSP buffer - Clamp health index in showDetail initial render (matches render() path) - Add service_id + node_id to poll staleness check (sequence alone can collide) - Abort fetchParam chain before issuing request when node changes (not just in callback) - Extract POLL_INTERVAL_MS / POLL_MAX_ATTEMPTS named constants - const writeValue (was let, never reassigned) - CSS: fix .health-badge 6-space indent; 40px bottom padding → 2.5em
- Extract getModeLabel() helper; replace 3 identical inline expressions - esc() applied to all firmware-sourced innerHTML fields (nodeID, vendor status, sw/hw version numbers) for consistency with XSS defence - fetchParam: abort at index > 254 (DroneCAN param index upper bound) - parseInt radix: add base 10 to saveConfig node ID parse - MSPHelper: delete forward-reference comment about future min/max fields
Decode min/max from async result MSP payload, show Range column in the params table, and block out-of-range writes with a red input outline and inline error on the Write button. Save to EEPROM button gets min-width to prevent layout shift on text change.
Add dronecan* locale keys for all param tab UI strings and wire them up in dronecan.js. Reuses the existing 'OK' key for write success.
…d, BigInt comparison - MSPHelper.js: define PARAM_TYPE_* constants; guard decodeNumeric() calls to INT/FLOAT params only (BOOL/STRING have no numeric range); replace magic numbers inside decodeNumeric with named constants - dronecan.js: use Number(param.min/max) in range comparison to keep validation in Number domain consistently with payload.value; fix TYPE_LABELS unknown-type fallback to use esc(String(...))
…s, BigInt comparison - dronecan.js: define PARAM_TYPE_INT/FLOAT/BOOL/STRING constants; replace all magic 1/2/3/4 literals throughout; fix BigInt range comparison using typeof guard so out-of-safe-integer bounds are compared in BigInt domain; i18n param table column headers; remove esc() from bundled i18n strings - locale/en/messages.json: add dronecanParamCol* keys adjacent to existing dronecan block
Non-numeric text in an INT/FLOAT param field produces NaN, which passes the range check silently then throws inside BigInt() in dronecanAsyncPoll, leaving the button permanently frozen. Add an isNaN guard that shows the existing out-of-range feedback instead.
…ope, NaN tooltip, CSS cleanup - dronecan.js: add DRONECAN_SERVICE_* constants; replace all bare service ID literals at call sites and in dronecanAsyncPoll guards; fix NaN tooltip to show dronecanParamOutOfRange instead of empty string - MSPHelper.js: hoist PARAM_TYPE_* constants to IIFE top scope; remove duplicate nested declarations from service_id===11 block - dronecan.css: scope health badge classes under #dronecan-node-table to avoid global name collisions; remove 2-space outer indentation
Mirror the service ID constants already defined in dronecan.js into MSPHelper's IIFE so the decode switch uses named constants throughout rather than bare literals with inline comments.
- Add DRONECAN_ASYNC_STATE_READY/ERROR and DRONECAN_ASYNC_REQUEST_STATUS_OK/BUSY named constants; replace bare 0/1/2/3 state literals in dronecanAsyncPoll - Add DRONECAN_EXECUTE_OPCODE_SAVE constant; replace bare 0 in Save to EEPROM handler - Fix param index safety cap: 254→8191 to match UAVCAN v0 uint13 spec; correct comment - Fix INT write precision: use BigInt(writeValue) directly instead of Number() to avoid silent loss of precision for values outside Number.MAX_SAFE_INTEGER - Update INT range validation to compare in BigInt domain throughout - Update BigInt encode to handle pre-BigInt payload.value without Math.trunc() - i18n: replace 's ago' string literals with dronecanSecondsAgo message key - i18n: replace hardcoded 'Error' in Name cell with dronecanNameError message key - Add dronecanSecondsAgo and dronecanNameError keys to locale/en/messages.json
7 tasks
…ht#2671 Fix 3 BUG-severity accessibility issues (missing <label> associations, missing table header), refactor two CRITICAL-complexity functions (cognitive complexity 23 and 32, both over the 15 threshold) by extracting helper functions, flatten function nesting that exceeded 4 levels, remove nested ternaries and a btn-aliasing pattern, and apply optional chaining in place of manual null checks. Also fix WCAG contrast failures on two button classes (~2.7:1 -> ~6:1) and address MINOR style findings (Number.parseInt/parseFloat/isNaN, String#replaceAll, String.fromCodePoint, redundant block, trailing .0 literals).
…ue helpers paramValueToString deduplicated a repeated pattern but kept the nested ternary it was meant to eliminate; convertParamValue's catch still returned the bare global NaN instead of Number.NaN. Both were newly flagged by SonarCloud after the previous commit. Verified with eslint-plugin-sonarjs (cognitive-complexity) locally: no violations.
4 tasks
… branch Add English fallback text to the bitrate/node-ID labels and save button (Web:S6853 x2, Web:S6827) so accessible names exist before i18next populates them at runtime, matching the fallback-text convention already used elsewhere (gpsPresetMode, gpsUpdateRate, and the a11y fix on fix/dronecan-gps-health-guard). In MSPHelper.js's async-result decoder: use String.fromCodePoint instead of fromCharCode for the two DroneCAN string fields (javascript:S7758 x2, behaviourally identical here since decoded bytes are single code units); consolidate decodeNumeric()'s three divergent return statements into a single return point (javascript:S3800), preserving the existing number/bigint/undefined value semantics that dronecan.js's min/max consumers depend on; and log the truncated/malformed-response catch instead of swallowing it silently (javascript:S2486). Verified against test_dronecan_async_result.mjs (68/68 passing).
|
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
1 participant
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.
Summary
Adds a DroneCAN configurator tab UI for on-demand node management: GetNodeInfo, parameter Get/Set (with range validation), ExecuteOpcode, and RestartNode, driven via async MSP slot.
Companion to firmware PR iNavFlight/inav#11683 — the two are linked and should be reviewed/merged together.
Test plan