Add support for ICM45605 and ICM45686 #4432
Conversation
✅ Deploy Preview for origin-betaflight-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
haslinghuis
force-pushed
the
add-icm45686
branch
from
fe904b3 to
edcde22
Compare
haslinghuis
changed the title
haslinghuis
requested review from
ledvinap,
KarateBrot,
nerdCopter and
VitroidFPV
haslinghuis
changed the title
WalkthroughThe changes update how new sensor types are inserted into sensor arrays based on the API version. The utility function used for array insertion is switched to allow adding multiple elements at once. Specifically, three new sensor types are now inserted simultaneously after a specific existing sensor type, instead of adding just one. Other logic for removing deprecated sensors and adding virtual sensors remains unchanged. Changes
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Preview URL: https://ca6433c8.betaflight-configurator.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/sensor_types.js(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/js/sensor_types.js (1)
src/js/utils/array.js (2)
addArrayElementsAfter(35-43)removeArrayElement(7-12)
🔇 Additional comments (3)
src/js/sensor_types.js (3)
4-4: Import updated appropriately.The import statement has been correctly updated to include the new
addArrayElementsAfterutility function while retaining the necessaryremoveArrayElementandaddArrayElementfunctions.
114-114: Improved code maintainability for gyro sensors.The change consolidates what would have been multiple single-element additions into one function call, making the code more maintainable and easier to extend in the future when additional sensors need to be added.
120-120: Improved code maintainability for accelerometer sensors.Similar to the gyro sensors change, this consolidates multiple operations into a single function call, improving code consistency and maintainability.
src/js/sensor_types.js
Outdated
| addArrayElementsAfter(gyroElements, "LSM6DSV16X", ["IIM42653", "ICM45686", "ICM45605"]); | ||
|
|
||
| removeArrayElement(accElements, "ADXL345"); | ||
| removeArrayElement(accElements, "MMA8452"); | ||
| removeArrayElement(accElements, "BMA280"); | ||
| removeArrayElement(accElements, "LSM303DLHC"); | ||
| addArrayElementAfter(accElements, "LSM6DSV16X", "IIM42653"); | ||
| addArrayElementsAfter(accElements, "LSM6DSV16X", ["IIM42653", "ICM45686", "ICM45605"]); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify sensor detection works correctly.
The addition of the new sensor types (ICM45686 and ICM45605) alongside IIM42653 should be verified to ensure proper sensor detection in the actual application.
🏁 Script executed:
#!/bin/bash
# Search for any regression tests that might verify these sensor type integrations
fd -t f -e js test | xargs grep -l "ICM45686\|ICM45605" || echo "No test files found containing the new sensor types"
# Look for implementation files that might be using these new sensor types
fd -t f -e js src | xargs grep -l "ICM45686\|ICM45605" | grep -v sensor_types.js || echo "No additional implementation files found using these sensor types"Length of output: 394
Add regression tests for ICM45686 and ICM45605
No existing tests reference the newly added sensor types. Please add unit or integration tests to verify that both gyroElements and accElements include “IIM42653”, “ICM45686”, and “ICM45605” immediately after “LSM6DSV16X”.
• Location to cover:
- src/js/sensor_types.js (around lines 114–120)
• What to test: - That addArrayElementsAfter correctly injects the three new sensors in the proper order for both
gyroElementsandaccElements.
• Example snippet for a new test file (e.g.test/sensor_types.test.js):
import { gyroElements, accElements } from '../src/js/sensor_types';
describe('sensor_types array augmentation', () => {
it('adds new gyro sensors after LSM6DSV16X', () => {
const idx = gyroElements.indexOf('LSM6DSV16X');
expect(gyroElements.slice(idx + 1, idx + 4))
.toEqual(['IIM42653', 'ICM45686', 'ICM45605']);
});
it('adds new accelerometer sensors after LSM6DSV16X', () => {
const idx = accElements.indexOf('LSM6DSV16X');
expect(accElements.slice(idx + 1, idx + 4))
.toEqual(['IIM42653', 'ICM45686', 'ICM45605']);
});
});
haslinghuis
force-pushed
the
add-icm45686
branch
2 times, most recently
from
e64befd to
7f831d5
Compare
|
|
Preview URL: https://c9305682.betaflight-configurator.pages.dev |
|
Rebased |
This pull request updates the
sensorTypesfunction insrc/js/sensor_types.jsto enhance the handling of array modifications by replacing single-element additions with multi-element additions. It also updates the utility function used for this purpose.Updates to array modification logic:
addArrayElementAfterwithaddArrayElementsAfterto allow adding multiple elements at once in thegyroElementsandaccElementsarrays. This simplifies the code and improves extensibility.Utility function update:
addArrayElementsAfterfromutils/arrayinstead ofaddArrayElementAfter, reflecting the new multi-element addition approach.Summary by CodeRabbit
New Features
Bug Fixes