Feature: Add MSP AOA sensor support for relaxed static stability aircraft control#11484
Open
Miki4751 wants to merge 5 commits intoiNavFlight:masterfrom
Open
Feature: Add MSP AOA sensor support for relaxed static stability aircraft control#11484Miki4751 wants to merge 5 commits intoiNavFlight:masterfrom
Miki4751 wants to merge 5 commits intoiNavFlight:masterfrom
Conversation
…rcraft control - Add AOA sensor driver framework with MSP and virtual sensor support - Add sideslip angle field to MSP sensor message - Add NORMAL/CANARD aircraft layout support with gradual intervention - Add AOA sensor status display in CLI diagnostics - Add OSD elements for AOA display - Add logic condition support for AOA values - Add fw_aoa_intervention_threshold parameter (0-100%, default 80%)
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
Contributor
Review Summary by QodoAdd MSP AOA sensor support for relaxed static stability aircraft control WalkthroughsDescription• Add AOA sensor driver framework with MSP and virtual sensor support • Implement relaxed static stability aircraft control for NORMAL and CANARD layouts • Add gradual AOA intervention with configurable threshold for conventional aircraft • Integrate AOA data into OSD, logic conditions, and CLI diagnostics • Add MSP protocol support for AOA sensor data reception Diagramflowchart LR
MSP["MSP Protocol"] -->|"AOA Data"| AOAMSP["AOA MSP Driver"]
AOAMSP -->|"Virtual Interface"| AOASENSOR["AOA Sensor Core"]
AOASENSOR -->|"Raw AOA/Sideslip"| AOAPROCESS["AOA Processing"]
AOAPROCESS -->|"Filtered Data"| CONTROL["AOA Control Logic"]
CONTROL -->|"NORMAL Layout"| GRADUAL["Gradual Intervention"]
CONTROL -->|"CANARD Layout"| DIRECT["Direct Servo Control"]
GRADUAL -->|"PID Output"| SERVOS["Servo Mixer"]
DIRECT -->|"PID Output"| SERVOS
AOAPROCESS -->|"Display Data"| OSD["OSD/CLI/Logic Conditions"]
File Changes1. src/main/drivers/aoa/aoa.h
|
Contributor
Code Review by Qodo
|
Author
|
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+72
to
+77
| void mspAoaReceiveNewData(uint8_t * bufferPtr) | ||
| { | ||
| const mspSensorAoaDataMessage_t * pkt = (const mspSensorAoaDataMessage_t *)bufferPtr; | ||
| sensorAoa = pkt->aoa; | ||
| sensorSideslip = pkt->sideslip; | ||
| hasNewData = true; |
Contributor
There was a problem hiding this comment.
1. mspaoareceivenewdata() lacks size validation 📘 Rule violation ⛨ Security
The new MSP AOA sensor handler casts and reads fields from an incoming buffer without validating the payload length, which can cause out-of-bounds reads/crashes on malformed or truncated packets. This violates the requirement to validate external protocol/payload sizes before use.
Agent Prompt
## Issue description
The MSP AOA sensor message handler reads `aoa` and `sideslip` from an incoming buffer without validating the payload length, risking out-of-bounds reads on malformed/truncated packets.
## Issue Context
`mspProcessSensorCommand()` computes `dataSize` but the `MSP2_SENSOR_AOA` path does not validate it or pass it into `mspAoaReceiveNewData()`. `mspAoaReceiveNewData()` then casts `bufferPtr` to `mspSensorAoaDataMessage_t` and dereferences fields unconditionally.
## Fix Focus Areas
- src/main/fc/fc_msp.c[4440-4491]
- src/main/io/aoa.h[32-34]
- src/main/io/aoa_msp.c[72-78]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+146
to
+155
| bool aoaProcess(void) | ||
| { | ||
| if (aoaSensor.dev.read) { | ||
| int16_t rawAoa, rawSideslip; | ||
| aoaSensor.dev.read(&aoaSensor.dev, &rawAoa, &rawSideslip); | ||
| int16_t aoaWithOffset = rawAoa + DEGREES_TO_DECIDEGREES(aoaConfig()->aoa_offset); | ||
| aoaSensor.aoa = constrain(aoaWithOffset, DEGREES_TO_DECIDEGREES(aoaConfig()->aoa_min_angle), DEGREES_TO_DECIDEGREES(aoaConfig()->aoa_max_angle)); | ||
| aoaSensor.sideslip = rawSideslip; | ||
| aoaSensor.lastValidResponseTimeMs = millis(); | ||
| DEBUG_SET(DEBUG_AOA, 0, aoaSensor.aoa); |
Contributor
There was a problem hiding this comment.
2. aoaprocess() mishandles aoa_no_new_data 📘 Rule violation ☼ Reliability
aoaProcess() constrains and publishes rawAoa even when it is the no-data sentinel (AOA_NO_NEW_DATA), and it updates lastValidResponseTimeMs regardless, making stale/invalid data appear fresh and healthy. This violates the requirement to use consistent sentinel values and avoid stale partial data on failed/invalid reads.
Agent Prompt
## Issue description
`AOA_NO_NEW_DATA` is not handled as a sentinel: it gets constrained into a valid AOA range and the sensor health timestamp is refreshed, which can mask missing/stale data.
## Issue Context
The MSP AOA driver initializes `sensorAoa`/`sensorSideslip` to `AOA_NO_NEW_DATA`. `aoaProcess()` reads these values, applies offset/constrain, and updates `lastValidResponseTimeMs` unconditionally.
## Fix Focus Areas
- src/main/drivers/aoa/aoa.h[33-37]
- src/main/io/aoa_msp.c[40-78]
- src/main/sensors/aoa.c[146-173]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+39
to
+49
| static void virtualAoaInit(aoaDev_t * dev) | ||
| { | ||
| UNUSED(dev); | ||
| return highLevelDeviceVTable->init(); | ||
| } | ||
|
|
||
| static void virtualAoaUpdate(aoaDev_t * dev) | ||
| { | ||
| UNUSED(dev); | ||
| return highLevelDeviceVTable->update(); | ||
| } |
Contributor
There was a problem hiding this comment.
3. Void return expression 🐞 Bug ≡ Correctness
virtualAoaInit/virtualAoaUpdate are declared void but use `return highLevelDeviceVTable->...()` which is not valid ISO C and can fail compilation (or at least emit warnings) on stricter toolchains.
Agent Prompt
### Issue description
`virtualAoaInit()` and `virtualAoaUpdate()` are `void` functions but currently use `return highLevelDeviceVTable->init();` / `return highLevelDeviceVTable->update();`, which is not valid ISO C and may break builds on stricter embedded toolchains.
### Issue Context
These functions are part of the newly added AOA virtual device adapter and are called through the AOA device vtable.
### Fix Focus Areas
- src/main/drivers/aoa/aoa_virtual.c[39-49]
### Expected fix
Change to simple calls:
- `highLevelDeviceVTable->init(); return;` (or just call without `return`)
- `highLevelDeviceVTable->update(); return;`
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Miki4751
force-pushed
the
clean/aoa-sensor
branch
from
eec19c1 to
78751d6
Compare
Miki4751
force-pushed
the
clean/aoa-sensor
branch
from
78751d6 to
3bb08ee
Compare
Member
|
Thanks for this! It looks like you put a lot of work into it. This was something I had though about doing and I designed a couple of miniature Hall-effect AoA sensors but I never did anything with them. There may some potential areas of improvement possible. There are some things to look at from a coding perspective, and from an aerodynamics and safety perspective. Also, as-is, the code doesn't compile. I'll make two separate comments for code vs aerodynamics and feature design of the MCAS. Before really digging into the code myself, here are some things my agent thought might be worth looking at. Sometimes the agent is wrong and sometimes we make different design decisions than the agent recommends, but these are things that might be worth CHECKING to see if the agent is pointing out a real issue or not: Additional concern to check: |
Member
|
If the aircraft is already pitched 12 degrees nose down, should we force it further nose down? To avoid the catastrophic failure modes of the Boeing's infamous MCAS, we might do something like this to compare the current pitch and throttle vs AoA. |
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
2 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.
Summary
This PR adds angle-of-attack (AOA) sensor functionality via the MSP protocol, specifically designed for control of relaxed static stability (RSS) aircraft.
Key Features
For Canard-configured Aircraft
For Conventional-configured Aircraft
fw_aoa_intervention_thresholdparameter (0-100%, default 80%)Additional Features
Configuration
All settings can be configured via CLI:
aoa_hardware- AOA sensor type (NONE/MSP/FAKE)aoa_offset- Calibration offset in degreesaoa_max_angle/aoa_min_angle- AOA range limitsfw_aoa_control_channel- Enable channel (-1 = always enabled)fw_aoa_upper_limit_angle- Maximum positive AOA limitfw_aoa_lower_limit_angle- Minimum negative AOA limitfw_aoa_aircraft_type- NORMAL or CANARD layoutfw_aoa_kp- P gain percentagefw_aoa_intervention_threshold- Intervention threshold percentageValidation
This feature has been extensively validated over a 5-month period using a SpeedyBee F405 flight controller installed in a Freewing J-10 aircraft, with all performance targets fully met.
Future Work
Further optimization is planned to implement optimal AOA control for cruise flight, improving overall efficiency and performance.