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. |
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
|
Code Review by Qodo
1.
|
|
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| static void virtualAoaInit(aoaDev_t * dev) | ||
| { | ||
| UNUSED(dev); | ||
| return highLevelDeviceVTable->init(); | ||
| } | ||
|
|
||
| static void virtualAoaUpdate(aoaDev_t * dev) | ||
| { | ||
| UNUSED(dev); | ||
| return highLevelDeviceVTable->update(); | ||
| } |
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
eec19c1 to
78751d6
Compare
78751d6 to
3bb08ee
Compare
|
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: |
|
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. |
|
Any thoughts, @Miki4751 ? |
- Gate pitch control output when no valid AOA data received (startup runaway) - Validate MSP2_SENSOR_AOA payload size before parsing - Append AOA byte to end of MSP_SENSOR_CONFIG and fix read-side overflow - Align SENSOR_AOA bitmask with SENSOR_INDEX_AOA - Remove dead parameters from aoaControlUpdate(), use float CANARD limits - Round PID/offset float to int16_t with lrintf() to avoid truncation - Fix aoaConfig_t comment units (degrees), document CRSF AOA frame layout - Restore OPFLOW task period comment
0eb352c to
d74fea8
Compare
Thanks for taking the time to review this, and for the kind words! I have fixed the flagged issues and the code compiles cleanly now. The only item I did not change is point 2 — that was a deliberate decision rather than an oversight. The AOA sensor output jitters noticeably in practice. If the correction loop responded to every 0.1° fluctuation, the servo would be in near-continuous motion, resulting in very high servo load. Based on actual use, only commanding a surface adjustment once the AOA difference exceeds 1° has no adverse impact on control — the deadband filters out sensor noise while still intervening early enough to be effective. |
|
On a side note: the AOA telemetry is already usable end-to-end — I have verified receiving the AOA values on an EdgeTX TX16S radio, which makes pilot-facing feedback easy to set up, e.g. audible/verbal alerts for excessive AOA or approaching stall. |
…after AOA offset - Zero GVAR (aoaPidOutput) when AOA control is disabled by RC switch, consistent with the FAILING early-exit path - Constrain pitch PID output to +/-limit after adding the AOA offset, matching the canard branch intervention pattern - Drop DEBUG_AOA[5] reference to undeclared interventionOffset in the normal-layout branch (compile error)
|
I had my AI-assisted tool look at this and some questions were raised. Can you check if the following issues might be real? It also confirmed the CRITICAL startup pitch-control runaway and MSP buffer overread from earlier rounds look fixed in the current head, along with the sensor-bitmask misalignment, 1. Control-direction sign — is this inverted? Both branches seem to add a positive pitch offset for high AOA: 2. Is there a sensor-fault cross-check anywhere? I didn't find anything in A few smaller things, non-blocking:
Happy to take another look once you've had a chance to check these — please correct me on anything I've misread. |
2 participants
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.