refactor(bus): naming cleanup phase 2 (type sweep + identifier consistency)#1111
Merged
nerdCopter merged 3 commits intoemuflight:masterfrom Apr 21, 2026
Merged
Conversation
Stage G of the extDevice_t refactor, phase 2 branch.
bus.h: primary struct tag renamed busDevice_s -> extDevice_s, primary typedef
produces extDevice_t, mpuSlave master pointer typed const struct
extDevice_s *. Kept `typedef extDevice_t busDevice_t;` as a transitional
alias so any overlooked caller still compiles; the alias can be removed
in a follow-up once the whole tree is visibly clean.
Function declarations in bus.h updated to take const extDevice_t *dev
parameter names (body-local parameter-name rename in bus.c / bus_spi.c /
bus_i2c_busdev.c and callers is cosmetic and deferred to a follow-up).
Global sweep: 49 other source + unit-test files, 174 insertions / 174
deletions, line-for-line. Only busDevice_t references that remain are the
typedef alias line and its comment in bus.h.
No behavioural change. Verified: make test + 5 targets rc=0, 0 new compiler
warnings (HELIOSPRING FOXEERF405 FOXEERF722V4 HGLRCF411 NBDHMBF4PRO).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…res and bodies Stage H of the extDevice_t refactor, phase 2 branch. Mechanical identifier cleanup to match BF 4.5-maintenance's `const extDevice_t *dev` convention. Two passes: 1. `busdev` global rename (not an English word, safe). 19 files, renamed both active code and commented-out references for consistency. 2. `bus` syntactic-context rename. `bus` IS an English word, so used identifier-position sed patterns (\*bus\b / &bus\b / \bbus-> / (bus,) instead of \bbus\b globally. Comment uses like "serial bus", "APB1 bus", "SPI bus utilization" are preserved. 27 files. compass.c single-file exception: compassDetect(magDev_t *dev) already has a parameter named `dev`, so its local alias was renamed busdev -> extDev instead of -> dev to avoid shadow + self-referencing init. No behavioural change. 44 files, 733 insertions / 733 deletions (line-for-line). Verified: make test + 5 targets rc=0, 0 new compiler warnings vs Stage A baseline (HELIOSPRING FOXEERF405 FOXEERF722V4 HGLRCF411 NBDHMBF4PRO). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stage H-bis. After Stage H migrated every caller off busDevice_t, the only remaining reference was the transitional alias in bus.h itself. Remove it: nothing in the tree depends on the name any longer. bus.h shrinks by 7 lines (alias declaration + its explanatory comment block). Full grep of src/: zero `\bbusDevice_t\b` matches anywhere after this commit. The struct tag remains `struct extDevice_s`; the typedef remains `extDevice_t`. Verified: make test + 5 targets rc=0, 0 new compiler warnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request performs a widespread refactoring that renames the core device abstraction type from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/drivers/barometer/barometer_lps.c (1)
244-250: Use the localdevalias for the WHO_AM_I read.
lpsDetect()introducesdevfor this exact purpose, but Line 250 still reaches through&baro->dev. This is functionally equivalent, but it leaves one naming-consistency miss in the sweep.♻️ Proposed cleanup
- lpsReadCommand(&baro->dev, LPS_WHO_AM_I, &temp, 1); + lpsReadCommand(dev, LPS_WHO_AM_I, &temp, 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/drivers/barometer/barometer_lps.c` around lines 244 - 250, The WHO_AM_I read uses &baro->dev instead of the local alias dev introduced earlier; update the lpsReadCommand call in lpsDetect (replace lpsReadCommand(&baro->dev, LPS_WHO_AM_I, &temp, 1)) to use the local pointer (dev) so the call becomes lpsReadCommand(dev, LPS_WHO_AM_I, &temp, 1), keeping naming consistent with extDevice_t *dev and the preceding IO/ SPI setup.src/main/drivers/compass/compass_lis3mdl.c (1)
129-134: Use the localdevalias for the detect read.
lis3mdlDetect()sets updev, but Line 134 still passes&mag->dev. Keeping this asdevcloses a residual identifier-consistency gap.♻️ Proposed cleanup
- bool ack = busReadRegisterBuffer(&mag->dev, LIS3MDL_REG_WHO_AM_I, &sig, 1); + bool ack = busReadRegisterBuffer(dev, LIS3MDL_REG_WHO_AM_I, &sig, 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/drivers/compass/compass_lis3mdl.c` around lines 129 - 134, The detect routine lis3mdlDetect sets up a local alias extDevice_t *dev = &mag->dev but the subsequent bus read still uses &mag->dev; change the call to busReadRegisterBuffer to use the local alias (dev) instead of &mag->dev so identifier usage is consistent: replace the argument passed to busReadRegisterBuffer(..., LIS3MDL_REG_WHO_AM_I, &sig, 1) from &mag->dev to dev, keeping the rest of the call and variables (sig, LIS3MDL_REG_WHO_AM_I, busReadRegisterBuffer) unchanged.src/main/drivers/compass/compass_hmc5883l.c (1)
217-233: Minor: use the localdevconsistently throughouthmc5883lDetect.Lines 222 and 230 still pass
&mag->deveven though the function already hasextDevice_t *dev = &mag->dev;on line 218 (used on lines 221/226/227). Functionally equivalent, but the mixed usage is easy to miss in future reads and defeats the point of the local.♻️ Proposed fix
`#ifdef` USE_MAG_SPI_HMC5883 if (dev->busType == BUS_TYPE_SPI) { - hmc5883SpiInit(&mag->dev); + hmc5883SpiInit(dev); } `#endif` `#ifdef` USE_MAG_HMC5883 if (dev->busType == BUS_TYPE_I2C && dev->busType_u.i2c.address == 0) { dev->busType_u.i2c.address = HMC5883_MAG_I2C_ADDRESS; } `#endif` - bool ack = busReadRegisterBuffer(&mag->dev, HMC58X3_REG_IDA, &sig, 1); + bool ack = busReadRegisterBuffer(dev, HMC58X3_REG_IDA, &sig, 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/drivers/compass/compass_hmc5883l.c` around lines 217 - 233, In hmc5883lDetect, consistently use the local pointer dev instead of dereferencing mag->dev in the calls; replace occurrences of &mag->dev passed to hmc5883SpiInit and busReadRegisterBuffer with dev so hmc5883SpiInit(dev) and busReadRegisterBuffer(dev, HMC58X3_REG_IDA, &sig, 1) use the already-defined extDevice_t *dev = &mag->dev, keeping usage uniform with the other references to dev in this function.src/main/drivers/accgyro/accgyro_spi_mpu9250.c (1)
75-83: Dead-code initializer references a non-existent field.Per
bus.h,extDevice_thas no.spimember — SPI fields are nested under.busType_u.spi. The#if 0guard keeps this from breaking the build, but since the XXX comment and the type were updated as part of this sweep, the initializer should either be corrected for consistency or the stub removed. As-is, anyone who enables this block for testing will immediately hit a compile error that's unrelated to whatmpu9250SpiResetGyrois trying to demonstrate.♻️ Proposed fix (correct the designator)
`#ifdef` MPU9250_CS_PIN - extDevice_t dev = { .spi = { .csnPin = IOGetByTag(IO_TAG(MPU9250_CS_PIN)) } }; + extDevice_t dev = { .busType = BUS_TYPE_SPI, .busType_u.spi.csnPin = IOGetByTag(IO_TAG(MPU9250_CS_PIN)) }; mpu9250SpiWriteRegister(&dev, MPU_RA_PWR_MGMT_1, MPU9250_BIT_RESET);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/drivers/accgyro/accgyro_spi_mpu9250.c` around lines 75 - 83, The dead code block uses an incorrect designator for extDevice_t (it references .spi which no longer exists per bus.h); either remove the stub or update the initializer to use the correct nested field (busType_u.spi) so it compiles if enabled — e.g., change the initializer used when building an extDevice_t for mpu9250SpiWriteRegister/MPU_RA_PWR_MGMT_1/MPU9250_BIT_RESET (or drop the whole block and keep the comment) and ensure mpu9250SpiResetGyro callers use the corrected extDevice_t layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/drivers/max7456.c`:
- Around line 193-194: The globals max7456BusDevice and dev are currently
external and collide with identical symbols in another compilation unit; mark
both declarations in this file as static to give them internal linkage (e.g.,
change extDevice_t max7456BusDevice; and extDevice_t *dev = &max7456BusDevice;
to static declarations). Also consider renaming the generic pointer symbol dev
to a file-local name like max7456Dev (still static) to further avoid accidental
collisions and improve clarity; update all uses of dev and max7456BusDevice in
this file accordingly.
In `@src/main/drivers/nbd7456.c`:
- Around line 193-194: The file-scope variables max7456BusDevice and dev
currently have external linkage and should be made internal to avoid linker
collisions; change the declarations of max7456BusDevice and the pointer dev in
nbd7456.c (and the identical pair in max7456.c) to use static (i.e., make
max7456BusDevice static and dev static pointing at &max7456BusDevice) so both
symbols have internal linkage and cannot collide across translation units.
---
Nitpick comments:
In `@src/main/drivers/accgyro/accgyro_spi_mpu9250.c`:
- Around line 75-83: The dead code block uses an incorrect designator for
extDevice_t (it references .spi which no longer exists per bus.h); either remove
the stub or update the initializer to use the correct nested field
(busType_u.spi) so it compiles if enabled — e.g., change the initializer used
when building an extDevice_t for
mpu9250SpiWriteRegister/MPU_RA_PWR_MGMT_1/MPU9250_BIT_RESET (or drop the whole
block and keep the comment) and ensure mpu9250SpiResetGyro callers use the
corrected extDevice_t layout.
In `@src/main/drivers/barometer/barometer_lps.c`:
- Around line 244-250: The WHO_AM_I read uses &baro->dev instead of the local
alias dev introduced earlier; update the lpsReadCommand call in lpsDetect
(replace lpsReadCommand(&baro->dev, LPS_WHO_AM_I, &temp, 1)) to use the local
pointer (dev) so the call becomes lpsReadCommand(dev, LPS_WHO_AM_I, &temp, 1),
keeping naming consistent with extDevice_t *dev and the preceding IO/ SPI setup.
In `@src/main/drivers/compass/compass_hmc5883l.c`:
- Around line 217-233: In hmc5883lDetect, consistently use the local pointer dev
instead of dereferencing mag->dev in the calls; replace occurrences of &mag->dev
passed to hmc5883SpiInit and busReadRegisterBuffer with dev so
hmc5883SpiInit(dev) and busReadRegisterBuffer(dev, HMC58X3_REG_IDA, &sig, 1) use
the already-defined extDevice_t *dev = &mag->dev, keeping usage uniform with the
other references to dev in this function.
In `@src/main/drivers/compass/compass_lis3mdl.c`:
- Around line 129-134: The detect routine lis3mdlDetect sets up a local alias
extDevice_t *dev = &mag->dev but the subsequent bus read still uses &mag->dev;
change the call to busReadRegisterBuffer to use the local alias (dev) instead of
&mag->dev so identifier usage is consistent: replace the argument passed to
busReadRegisterBuffer(..., LIS3MDL_REG_WHO_AM_I, &sig, 1) from &mag->dev to dev,
keeping the rest of the call and variables (sig, LIS3MDL_REG_WHO_AM_I,
busReadRegisterBuffer) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ffffd83-a7cc-499d-b0e1-ad1a955825e4
📒 Files selected for processing (51)
src/main/drivers/accgyro/accgyro_imuf9001.csrc/main/drivers/accgyro/accgyro_imuf9001.hsrc/main/drivers/accgyro/accgyro_mpu.csrc/main/drivers/accgyro/accgyro_mpu.hsrc/main/drivers/accgyro/accgyro_spi_bmi160.csrc/main/drivers/accgyro/accgyro_spi_bmi160.hsrc/main/drivers/accgyro/accgyro_spi_bmi270.csrc/main/drivers/accgyro/accgyro_spi_bmi270.hsrc/main/drivers/accgyro/accgyro_spi_icm20649.csrc/main/drivers/accgyro/accgyro_spi_icm20649.hsrc/main/drivers/accgyro/accgyro_spi_icm20689.csrc/main/drivers/accgyro/accgyro_spi_icm20689.hsrc/main/drivers/accgyro/accgyro_spi_icm426xx.csrc/main/drivers/accgyro/accgyro_spi_icm426xx.hsrc/main/drivers/accgyro/accgyro_spi_mpu6000.csrc/main/drivers/accgyro/accgyro_spi_mpu6000.hsrc/main/drivers/accgyro/accgyro_spi_mpu6500.csrc/main/drivers/accgyro/accgyro_spi_mpu6500.hsrc/main/drivers/accgyro/accgyro_spi_mpu9250.csrc/main/drivers/accgyro/accgyro_spi_mpu9250.hsrc/main/drivers/barometer/barometer_bmp085.csrc/main/drivers/barometer/barometer_bmp280.csrc/main/drivers/barometer/barometer_lps.csrc/main/drivers/barometer/barometer_ms5611.csrc/main/drivers/barometer/barometer_qmp6988.csrc/main/drivers/bus.csrc/main/drivers/bus.hsrc/main/drivers/bus_i2c_busdev.csrc/main/drivers/bus_i2c_busdev.hsrc/main/drivers/bus_spi.csrc/main/drivers/bus_spi.hsrc/main/drivers/compass/compass_ak8963.csrc/main/drivers/compass/compass_ak8975.csrc/main/drivers/compass/compass_hmc5883l.csrc/main/drivers/compass/compass_lis3mdl.csrc/main/drivers/compass/compass_qmc5883l.csrc/main/drivers/display_ug2864hsweg01.csrc/main/drivers/display_ug2864hsweg01.hsrc/main/drivers/flash.csrc/main/drivers/flash_m25p16.csrc/main/drivers/flash_w25m.csrc/main/drivers/max7456.csrc/main/drivers/nbd7456.csrc/main/drivers/rx/rx_spi.csrc/main/io/dashboard.csrc/main/sensors/compass.csrc/main/sensors/gyro.csrc/main/sensors/gyro.hsrc/test/unit/baro_bmp085_unittest.cc.txtsrc/test/unit/baro_bmp280_unittest.ccsrc/test/unit/baro_ms5611_unittest.cc
Comment on lines
+193
to
+194
| extDevice_t max7456BusDevice; | ||
| extDevice_t *dev = &max7456BusDevice; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any file-scope definition of `dev` with extDevice_t/busDevice_t type, and external references to max7456BusDevice.
rg -nP --type=c --type=cpp -C1 '^\s*(extern\s+)?(ext|bus)Device_t\s*\*?\s*dev\s*(=|;)'
echo '---'
rg -nP -C1 '\bmax7456BusDevice\b'Repository: emuflight/EmuFlight
Length of output: 7096
Mark max7456BusDevice and dev static to avoid ODR violations and symbol collisions.
Both max7456BusDevice and dev are declared with external linkage in this file (lines 193–194). This same pair is also declared identically in src/main/drivers/nbd7456.c at lines 193–194, creating an ODR violation if both files are linked together. Additionally, dev is an extremely generic name that creates linker collision risk. Since both variables are used only within their respective files, they must be declared static.
♻️ Proposed fix
-extDevice_t max7456BusDevice;
-extDevice_t *dev = &max7456BusDevice;
+static extDevice_t max7456BusDevice;
+static extDevice_t *dev = &max7456BusDevice;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| extDevice_t max7456BusDevice; | |
| extDevice_t *dev = &max7456BusDevice; | |
| static extDevice_t max7456BusDevice; | |
| static extDevice_t *dev = &max7456BusDevice; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/drivers/max7456.c` around lines 193 - 194, The globals
max7456BusDevice and dev are currently external and collide with identical
symbols in another compilation unit; mark both declarations in this file as
static to give them internal linkage (e.g., change extDevice_t max7456BusDevice;
and extDevice_t *dev = &max7456BusDevice; to static declarations). Also consider
renaming the generic pointer symbol dev to a file-local name like max7456Dev
(still static) to further avoid accidental collisions and improve clarity;
update all uses of dev and max7456BusDevice in this file accordingly.
Member
Author
There was a problem hiding this comment.
@coderabbitai, this is a refactor-only PR. This seems to be a pre-existing issue.
There was a problem hiding this comment.
@nerdCopter, you're correct — the diff confirms this PR only performs a mechanical type rename (busDevice_t → extDevice_t) and identifier rename (busdev → dev). The missing static on max7456BusDevice and dev (and the corresponding issue in nbd7456.c) was already present before this PR and is outside its scope.
Would you like me to open a follow-up GitHub issue to track adding static to these declarations in both max7456.c and nbd7456.c?
🧠 Learnings used
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-04-04T21:55:50.478Z
Learning: In the EmuFlight project (GitHub: emuflight/EmuFlight), issue `#1106` involves CLI AutoComplete stalls on HELIOSPRING/STRIXF10/MODE2FLUX boards caused by USB VCP write starvation under EXTI preemption. The proposed fix (disabling EXTI via `EXTIEnable()` in `cliEnter()` and re-enabling in `cliExit()`/`cliRebootEx()`) was initially rejected due to apparent "permanent gyro corruption requiring reflash." However, this regression was later identified as a pre-existing configurator CLI/MSP reconnect bug present in the EmuFlight configurator NWjs master branch (not just an Electron/Forge migration bug). The configurator fails to properly re-establish MSP polling after CLI exits; a reflash incidentally forces USB re-enumeration which triggers a clean reconnect, masking the real cause. The EXTI fix itself is logically sound and should be re-evaluated using firmware-independent gyro verification (raw MSP tool) rather than the buggy configurator.
There was a problem hiding this comment.
@nerdCopter, thanks for sharing — #1112 is noted as the tracking issue for adding static to max7456BusDevice/dev in max7456.c and nbd7456.c. This review comment is resolved; the fix will be addressed as part of that follow-up.
🧠 Learnings used
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-04-04T21:55:50.478Z
Learning: In the EmuFlight project (GitHub: emuflight/EmuFlight), issue `#1106` involves CLI AutoComplete stalls on HELIOSPRING/STRIXF10/MODE2FLUX boards caused by USB VCP write starvation under EXTI preemption. The proposed fix (disabling EXTI via `EXTIEnable()` in `cliEnter()` and re-enabling in `cliExit()`/`cliRebootEx()`) was initially rejected due to apparent "permanent gyro corruption requiring reflash." However, this regression was later identified as a pre-existing configurator CLI/MSP reconnect bug present in the EmuFlight configurator NWjs master branch (not just an Electron/Forge migration bug). The configurator fails to properly re-establish MSP polling after CLI exits; a reflash incidentally forces USB re-enumeration which triggers a clean reconnect, masking the real cause. The EXTI fix itself is logically sound and should be re-evaluated using firmware-independent gyro verification (raw MSP tool) rather than the buggy configurator.
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-03-26T16:06:21.396Z
Learning: In the EmuFlight repository (emuflight/EmuFlight), the maintainer (nerdCopter) does not want follow-up GitHub issues created for GitHub Actions SHA pinning or cache key hardening suggestions. The preference is mutable version tags (e.g., v4, v5) and no issue tracking for this topic.
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 1099
File: .github/workflows/test.yml:21-38
Timestamp: 2026-03-26T14:11:55.128Z
Learning: In the EmuFlight repository (emuflight/EmuFlight), the maintainer prefers using mutable version tags (e.g., v4, v5) for GitHub Actions rather than pinning to full 40-character commit SHAs. Do not flag unpinned action versions as issues in this repository.
Comment on lines
+193
to
+194
| extDevice_t max7456BusDevice; | ||
| extDevice_t *dev = &max7456BusDevice; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no other TU exports or externs a global `dev` that would collide / cross-link.
rg -nP --type=c --type=cpp --type=h '^\s*(extern\s+)?(const\s+)?\w[\w\s\*]*\bdev\s*(=|;)' -g '!**/nbd7456.c'
# Also list all non-static file-scope `dev` pointer/struct globals across drivers.
rg -nP -C1 '^\s*extDevice_t\s*\*?\s*dev\b'Repository: emuflight/EmuFlight
Length of output: 9966
Make file-scope dev (and max7456BusDevice) static to avoid external-linkage collisions.
Both symbols are declared at file scope without static, so they have external linkage. Renaming busdev → dev is safe within this translation unit, but a global named dev is a linker-collision hazard — any other TU that later introduces a global dev will fail to link or cross-link silently through an extern declaration. These are only used inside nbd7456.c, so internal linkage is the correct scope. The same issue exists identically in src/main/drivers/max7456.c at the same lines.
♻️ Proposed fix
-extDevice_t max7456BusDevice;
-extDevice_t *dev = &max7456BusDevice;
+static extDevice_t max7456BusDevice;
+static extDevice_t *dev = &max7456BusDevice;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| extDevice_t max7456BusDevice; | |
| extDevice_t *dev = &max7456BusDevice; | |
| static extDevice_t max7456BusDevice; | |
| static extDevice_t *dev = &max7456BusDevice; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/drivers/nbd7456.c` around lines 193 - 194, The file-scope variables
max7456BusDevice and dev currently have external linkage and should be made
internal to avoid linker collisions; change the declarations of max7456BusDevice
and the pointer dev in nbd7456.c (and the identical pair in max7456.c) to use
static (i.e., make max7456BusDevice static and dev static pointing at
&max7456BusDevice) so both symbols have internal linkage and cannot collide
across translation units.
Member
Author
There was a problem hiding this comment.
@coderabbitai, this is a refactor-only PR. This seems to be a pre-existing issue.
There was a problem hiding this comment.
@nerdCopter, you're right — looking at the diff, the original code already had busDevice_t max7456BusDevice; and busDevice_t *busdev = &max7456BusDevice; without static before this PR. This refactor only renamed the type and identifier; the external-linkage concern predates these changes.
Would you like me to open a follow-up issue to track adding static to these globals in both nbd7456.c and max7456.c as a separate cleanup task?
🧠 Learnings used
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-04-04T21:55:50.478Z
Learning: In the EmuFlight project (GitHub: emuflight/EmuFlight), issue `#1106` involves CLI AutoComplete stalls on HELIOSPRING/STRIXF10/MODE2FLUX boards caused by USB VCP write starvation under EXTI preemption. The proposed fix (disabling EXTI via `EXTIEnable()` in `cliEnter()` and re-enabling in `cliExit()`/`cliRebootEx()`) was initially rejected due to apparent "permanent gyro corruption requiring reflash." However, this regression was later identified as a pre-existing configurator CLI/MSP reconnect bug present in the EmuFlight configurator NWjs master branch (not just an Electron/Forge migration bug). The configurator fails to properly re-establish MSP polling after CLI exits; a reflash incidentally forces USB re-enumeration which triggers a clean reconnect, masking the real cause. The EXTI fix itself is logically sound and should be re-evaluated using firmware-independent gyro verification (raw MSP tool) rather than the buggy configurator.
There was a problem hiding this comment.
@nerdCopter, thanks for opening #1112 to track the static linkage cleanup for max7456BusDevice / dev in both nbd7456.c and max7456.c. That keeps this refactor PR focused and defers the pre-existing concern to the right place.
🧠 Learnings used
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-04-04T21:55:50.478Z
Learning: In the EmuFlight project (GitHub: emuflight/EmuFlight), issue `#1106` involves CLI AutoComplete stalls on HELIOSPRING/STRIXF10/MODE2FLUX boards caused by USB VCP write starvation under EXTI preemption. The proposed fix (disabling EXTI via `EXTIEnable()` in `cliEnter()` and re-enabling in `cliExit()`/`cliRebootEx()`) was initially rejected due to apparent "permanent gyro corruption requiring reflash." However, this regression was later identified as a pre-existing configurator CLI/MSP reconnect bug present in the EmuFlight configurator NWjs master branch (not just an Electron/Forge migration bug). The configurator fails to properly re-establish MSP polling after CLI exits; a reflash incidentally forces USB re-enumeration which triggers a clean reconnect, masking the real cause. The EXTI fix itself is logically sound and should be re-evaluated using firmware-independent gyro verification (raw MSP tool) rather than the buggy configurator.
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-03-26T16:06:21.396Z
Learning: In the EmuFlight repository (emuflight/EmuFlight), the maintainer (nerdCopter) does not want follow-up GitHub issues created for GitHub Actions SHA pinning or cache key hardening suggestions. The preference is mutable version tags (e.g., v4, v5) and no issue tracking for this topic.
Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 1099
File: .github/workflows/test.yml:21-38
Timestamp: 2026-03-26T14:11:55.128Z
Learning: In the EmuFlight repository (emuflight/EmuFlight), the maintainer prefers using mutable version tags (e.g., v4, v5) for GitHub Actions rather than pinning to full 40-character commit SHAs. Do not flag unpinned action versions as issues in this repository.
4 tasks
This was referenced Apr 22, 2026
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.
Continuation of #1108 (phase 1, now merged).
What this does
Three commits that complete the identifier rename started in phase 1:
bb15493fa5) — sweepbusDevice_t→extDevice_tacross tree, keep aliasc7ee1f82e2) — renamebus/busdevparameter and body identifiers todev948ba74024) — drop the transitionalbusDevice_ttypedef aliasNo logic changes. After this series every driver signature and body uses
extDevice_t *devconsistently, matching BF 4.5-maintenance naming.Testing
Relationship to later phases
Phase 3 (
refactor/extDevice_t-bus-split) adds the bus-resource split infrastructure and is waiting for this to merge.Summary by CodeRabbit