Skip to content

Tube gyro pir #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ewowi wants to merge 6 commits into
base: main
Choose a base branch
from
+281 −15
Open

Tube gyro pir #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e3d35b
add MPU6050 driver
ewowi Feb 10, 2026
66f1d6d
MPU6050 driver to IMU driver
ewowi Feb 11, 2026
48e3647
Move I2C mgmt to Module IO (from IMU driver)
ewowi Feb 11, 2026
77d11ec
Module IO and IMU driver tweaks
ewowi Feb 11, 2026
c73ab86
Module IO I2S tweaks, IMU fix accell calculation
ewowi Feb 12, 2026
62de264
Merge branch 'main' into tube-pir-gyro
ewowi Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion platformio.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ extra_scripts =
scripts/save_elf.py
lib_deps =
ArduinoJson@>=7.0.0
elims/PsychicMqttClient@^0.2.4
elims/PsychicMqttClient@^0.2.4
ElectronicCats/MPU6050 @ 1.3.0 ; for D_IMU.h driver
; https://github.com/hanyazou/BMI160-Arduino.git ; hanyazou/BMI160-Arduino#057f36e002bee0473a54fcedf41b93acad059568 ; @ ^1.0.0 ; for BMI160

;💫
[moonlight]
Expand Down
98 changes: 97 additions & 1 deletion src/MoonBase/Modules/ModuleIO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#if FT_MOONBASE == 1

#include <Wire.h> // for i2C

#include "MoonBase/Module.h"
#include "driver/uart.h"

Expand Down Expand Up @@ -216,6 +218,15 @@ class ModuleIO : public Module {
addControl(rows, "Level", "text", 0, 32, true); // ro
addControl(rows, "DriveCap", "text", 0, 32, true); // ro
}

addControl(controls, "i2cFreq", "number", 0, 65534, false, "kHz");

control = addControl(controls, "i2cBus", "rows");
control["crud"] = "r";
rows = control["n"].to<JsonArray>();
{
addControl(rows, "address", "number", 0, 255, true); // ro
}
}

class PinAssigner {
Expand Down Expand Up @@ -564,6 +575,30 @@ class ModuleIO : public Module {
#else
pinAssigner.assignPin(16, pin_LED);
#endif
#ifdef CONFIG_IDF_TARGET_ESP32
pinAssigner.assignPin(21, pin_I2C_SDA);
pinAssigner.assignPin(22, pin_I2C_SCL);
#else
pinAssigner.assignPin(8, pin_I2C_SDA);
pinAssigner.assignPin(9, pin_I2C_SCL);
#endif
// In setBoardPresetDefaults() for board_none (default case):
#ifdef CONFIG_IDF_TARGET_ESP32
pinAssigner.assignPin(21, pin_I2C_SDA); // ESP32 classic
pinAssigner.assignPin(22, pin_I2C_SCL);
#elif defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32S3) || defined(CONFIG_IDF_TARGET_ESP32C3)
pinAssigner.assignPin(8, pin_I2C_SDA); // ESP32-C3
pinAssigner.assignPin(9, pin_I2C_SCL);
#elif defined(CONFIG_IDF_TARGET_ESP32C6)
pinAssigner.assignPin(23, pin_I2C_SDA); // ESP32-C6
pinAssigner.assignPin(22, pin_I2C_SCL);
#elif defined(CONFIG_IDF_TARGET_ESP32P4)
pinAssigner.assignPin(7, pin_I2C_SDA); // ESP32-P4 (common board default)
pinAssigner.assignPin(8, pin_I2C_SCL);
#else
pinAssigner.assignPin(21, pin_I2C_SDA); // Fallback
pinAssigner.assignPin(22, pin_I2C_SCL);
#endif
Comment on lines +578 to +601
Copy link

@coderabbitai coderabbitai bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Duplicate I2C pin assignments — first block is dead code.

Lines 578–584 assign I2C pins, then lines 586–601 immediately overwrite them with a more granular per-target mapping. The first block is redundant. For CONFIG_IDF_TARGET_ESP32, pins 21/22 are assigned twice; for other targets (e.g., C6), the first assignment (8/9) is silently overwritten.

Proposed fix — remove the first block 
-  `#ifdef` CONFIG_IDF_TARGET_ESP32
-      pinAssigner.assignPin(21, pin_I2C_SDA);
-      pinAssigner.assignPin(22, pin_I2C_SCL);
-  `#else`
-      pinAssigner.assignPin(8, pin_I2C_SDA);
-      pinAssigner.assignPin(9, pin_I2C_SCL);
-  `#endif`
-  // In setBoardPresetDefaults() for board_none (default case):
   `#ifdef` CONFIG_IDF_TARGET_ESP32
       pinAssigner.assignPin(21, pin_I2C_SDA);  // ESP32 classic
       pinAssigner.assignPin(22, pin_I2C_SCL);
🤖 Prompt for AI Agents 
In `@src/MoonBase/Modules/ModuleIO.h` around lines 578 - 601, Remove the redundant
initial I2C pin assignment block that calls pinAssigner.assignPin(21,
pin_I2C_SDA)/assignPin(22, pin_I2C_SCL) (the block guarded by `#ifdef`
CONFIG_IDF_TARGET_ESP32 at the top) because setBoardPresetDefaults() already
performs target-specific assignments; keep the later granular conditional block
(the `#ifdef` / `#elif` chain that includes CONFIG_IDF_TARGET_ESP32, ESP32S2/S3/C3,
ESP32C6, ESP32P4, and the fallback) and delete the earlier duplicate assignments
so pinAssigner.assignPin is only invoked once per board preset.


// trying to add more pins, but these pins not liked by esp32-d0-16MB ... 🚧
// pinAssigner.assignPin(4, pin_LED_02;
Expand Down Expand Up @@ -609,6 +644,8 @@ class ModuleIO : public Module {
newState["modded"] = true;
} else if (updatedItem.name == "usage") {
newState["modded"] = true;
} else if (updatedItem.name == "i2cFreq") {
Wire.setClock(updatedItem.value.as<uint16_t>() * 1000);
}

if (newState.size()) update(newState, ModuleState::update, _moduleName); // if changes made then update
Expand Down Expand Up @@ -701,6 +738,9 @@ class ModuleIO : public Module {
#endif // ethernet

#if FT_BATTERY
pinVoltage = UINT8_MAX;
pinCurrent = UINT8_MAX;
pinBattery = UINT8_MAX;
for (JsonObject pinObject : _state.data["pins"].as<JsonArray>()) {
uint8_t usage = pinObject["usage"];
if (usage == pin_Voltage) {
Expand Down Expand Up @@ -790,7 +830,37 @@ class ModuleIO : public Module {
}
#endif
}
}

pinI2CSDA = UINT8_MAX;
pinI2CSCL = UINT8_MAX;
for (JsonObject pinObject : _state.data["pins"].as<JsonArray>()) {
uint8_t usage = pinObject["usage"];
if (usage == pin_I2C_SDA) {
pinI2CSDA = pinObject["GPIO"];
EXT_LOGD(ML_TAG, "I2CSDA found %d", pinI2CSDA);
}
if (usage == pin_I2C_SCL) {
pinI2CSCL = pinObject["GPIO"];
EXT_LOGD(ML_TAG, "I2CSCL found %d", pinI2CSCL);
}
}

if (pinI2CSCL != UINT8_MAX && pinI2CSDA != UINT8_MAX) {
Wire.end(); // Clean up any previous I2C initialization
delay(100);
uint16_t frequency = _state.data["i2cFreq"];
if (Wire.begin(pinI2CSDA, pinI2CSCL, frequency * 1000)) {
EXT_LOGI(ML_TAG, "initI2C Wire sda:%d scl:%d freq:%d kHz", pinI2CSDA, pinI2CSCL, frequency);
// delay(200); // Give I2C bus time to stabilize
// Wire.setClock(50000); // Explicitly set to 100kHz
updateDevices();
} else
EXT_LOGE(ML_TAG, "initI2C Wire failed");
}
Comment on lines +848 to +859
Copy link

@coderabbitai coderabbitai bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Zero i2cFreq default will pass 0 Hz to Wire.begin().

The i2cFreq control has no "default" set in setupDefinition (line 222), so it defaults to 0. On line 852, frequency * 1000 evaluates to 0, and Wire.begin(sda, scl, 0) is called. While Arduino-ESP32's Wire.begin may internally fall back to 100 kHz when given 0, this behavior is implicit and undocumented. A user seeing "0 kHz" in the UI is also confusing.

Proposed fix — add a default and guard against zero

In setupDefinition, add a default:

-    addControl(controls, "i2cFreq", "number", 0, 65534, false, "kHz");
+    control = addControl(controls, "i2cFreq", "number", 10, 1000, false, "kHz");
+    control["default"] = 100;  // 100 kHz standard mode

And in readPins, guard:

     uint16_t frequency = _state.data["i2cFreq"];
+    if (frequency == 0) frequency = 100;  // fallback to 100 kHz
     if (Wire.begin(pinI2CSDA, pinI2CSCL, frequency * 1000)) {
🤖 Prompt for AI Agents 
In `@src/MoonBase/Modules/ModuleIO.h` around lines 848 - 859, The i2cFreq control
currently defaults to 0 and leads to calling Wire.begin(pinI2CSDA, pinI2CSCL, 0)
and showing "0 kHz" in the UI; update the setupDefinition to provide a sensible
default (e.g., 100) for i2cFreq and add a guard in the code that reads
_state.data["i2cFreq"] (the readPins/initI2C area) to treat 0 as the default
value (replace with 100 or another chosen default) before multiplying by 1000
and passing to Wire.begin(pinI2CSDA, pinI2CSCL, frequency * 1000); ensure any
logged frequency uses the adjusted value so the UI shows a nonzero kHz and
maintain behavior that falls back to a safe clock when frequency is absent or
zero.

} // readPins

uint8_t pinI2CSDA = UINT8_MAX;
uint8_t pinI2CSCL = UINT8_MAX;

#if FT_BATTERY
uint8_t pinVoltage = UINT8_MAX;
Expand Down Expand Up @@ -867,6 +937,32 @@ class ModuleIO : public Module {
#endif
}

void updateDevices() {
JsonDocument doc;
doc["i2cBus"].to<JsonArray>();
JsonObject newState = doc.as<JsonObject>();

EXT_LOGI(ML_TAG, "Scanning I2C bus...");
byte count = 0;
for (byte i = 1; i < 127; i++) {
Wire.beginTransmission(i);
if (Wire.endTransmission() == 0) {
JsonObject i2cDevice = newState["i2cBus"].as<JsonArray>().add<JsonObject>();
i2cDevice["address"] = i;

EXT_LOGI(ML_TAG, "Found I2C device at address 0x%02X", i);
count++;
}
}
EXT_LOGI(ML_TAG, "Found %d device(s)", count);
JsonObject i2cDevice = newState["i2cBus"].as<JsonArray>().add<JsonObject>();
i2cDevice["address"] = 255;
Comment on lines +958 to +959
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Sentinel entry with address 255 pollutes the device list.

A fake device at address 255 is appended after the real scan results. This is not a valid I2C address (7-bit range is 0–127) and will appear as a spurious device in the UI. If this is meant to ensure the i2cBus array is non-empty or signal end-of-list, consider handling that in the UI layer instead.

Proposed fix: remove the sentinel 
     EXT_LOGI(ML_TAG, "Found %d device(s)", count);
-    JsonObject i2cDevice = newState["i2cBus"].as<JsonArray>().add<JsonObject>();
-    i2cDevice["address"] = 255;
📝 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
JsonObject i2cDevice = newState["i2cBus"].as<JsonArray>().add<JsonObject>();
i2cDevice["address"] = 255;
// Lines 928-929 removed - sentinel entry deleted
🤖 Prompt for AI Agents 
In `@src/MoonBase/Modules/ModuleIO.h` around lines 928 - 929, Remove the sentinel
fake I2C device being appended with address 255: stop creating the JsonObject
via newState["i2cBus"].as<JsonArray>().add<JsonObject>() and assigning
i2cDevice["address"] = 255 so the i2cBus array only contains real scan results;
if a non-empty-array indicator is needed, handle that in the UI layer instead of
adding a sentinel entry (look for usages of i2cDevice/newState["i2cBus"] in
ModuleIO.h and related scan/serialize functions to ensure no other code depends
on the sentinel).


doc["i2cFreq"] = Wire.getClock() / 1000;

update(newState, ModuleState::update, _moduleName);
}

private:
ESP32SvelteKit* _sveltekit;
uint8_t current_board_id = UINT8_MAX;
Expand Down
3 changes: 3 additions & 0 deletions src/MoonBase/NodeManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ class NodeManager : public Module {
if (nodeClass != nullptr) {
nodeClass->on = updatedItem.value.as<bool>(); // set nodeclass on/off
// EXT_LOGD(ML_TAG, " nodeclass 🔘:%d 🚥:%d 💎:%d", nodeClass->on, nodeClass->hasOnLayout(), nodeClass->hasModifier());
xSemaphoreTake(*nodeClass->layerMutex, portMAX_DELAY);
nodeClass->onUpdate(updatedItem.oldValue, nodeState); // custom onUpdate for the node
Copy link

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent mutex handling and argument semantics for onUpdate.

Two observations:

  1. No mutex: The controls-value path (line 282–285) holds layerMutex around onUpdate, but this new on-toggle path does not. For MPU6050Driver the hardware init may not need it, but other nodes' onUpdate could mutate shared layer state. Consider wrapping consistently:
+            xSemaphoreTake(*nodeClass->layerMutex, portMAX_DELAY);
             nodeClass->onUpdate(updatedItem.oldValue, nodeState);  // custom onUpdate for the node
+            xSemaphoreGive(*nodeClass->layerMutex);
  1. Argument type mismatch: Here nodeState (full node JSON with name, on, controls) is passed as the control parameter, while at line 284 a single control object is passed. The MPU6050Driver relies on checking control["on"] to distinguish, but this convention is implicit and fragile — a future node's onUpdate could misinterpret the JSON shape.
🤖 Prompt for AI Agents 
In `@src/MoonBase/NodeManager.h` at line 267, The call to nodeClass->onUpdate
currently passes the full nodeState without holding layerMutex and thus is
inconsistent with the controls-value path; wrap the on-toggle path's
nodeClass->onUpdate call with layerMutex (same scope used in the controls-value
path) and normalize the argument semantics by passing a consistent control
object (e.g., construct a single-control JSON containing only the on field or an
explicit {"on": ...} object) instead of the full nodeState so onUpdate
implementations always receive the same JSON shape.

xSemaphoreGive(*nodeClass->layerMutex);
nodeClass->requestMappings();
} else
EXT_LOGW(ML_TAG, "Nodeclass %s not found", nodeState["name"].as<const char*>());
Expand Down
3 changes: 3 additions & 0 deletions src/MoonBase/Nodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,8 @@ static struct SharedData {
size_t connectedClients;
size_t activeClients;
size_t clientListSize;

Coord3D gravity;
Comment on lines +347 to +348
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Coord3D gravity may not be the right type for gravity data.

As noted in the D_IMU.h review, the MPU6050 gravity vector contains float values in the range [–1.0, 1.0]. If Coord3D uses integer fields (e.g., int16_t), this will silently truncate to 0/±1. Consider using a float-based struct (e.g., VectorFloat) or documenting the expected scale/encoding if you intend to pre-scale the values.

🤖 Prompt for AI Agents 
In `@src/MoonBase/Nodes.h` around lines 347 - 348, The field Coord3D gravity
likely uses integer components and will truncate the MPU6050 float gravity
vector; update the type or handling: replace Coord3D gravity with a float-based
struct (e.g., VectorFloat or a new Coord3DFloat) or store gravity as an array of
floats and ensure any code using gravity performs the proper float
reads/conversions (see related D_IMU.h handling), or if you must keep Coord3D,
add clear documentation and explicit scaling/quantization conversions where
gravity is assigned so values in [−1.0,1.0] are preserved correctly.

} sharedData;

/**
Expand All @@ -360,6 +362,7 @@ static struct SharedData {
#include "MoonLight/Nodes/Drivers/D_FastLED.h"
#include "MoonLight/Nodes/Drivers/D_Hub75.h"
#include "MoonLight/Nodes/Drivers/D_Infrared.h"
#include "MoonLight/Nodes/Drivers/D_IMU.h"
#include "MoonLight/Nodes/Drivers/D_ParallelLEDDriver.h"
#include "MoonLight/Nodes/Drivers/D__Sandbox.h"
#include "MoonLight/Nodes/Effects/E_FastLED.h"
Expand Down
2 changes: 2 additions & 0 deletions src/MoonLight/Modules/ModuleDrivers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class ModuleDrivers : public NodeManager {
addControlValue(control, getNameAndTags<ArtNetOutDriver>());
addControlValue(control, getNameAndTags<AudioSyncDriver>());
addControlValue(control, getNameAndTags<IRDriver>());
addControlValue(control, getNameAndTags<IMUDriver>());
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Build failure: IMUDriver is not declared — class is named MPU6050Driver in D_IMU.h.

This is the downstream effect of the class name mismatch in src/MoonLight/Nodes/Drivers/D_IMU.h (Line 16), where the class is defined as MPU6050Driver. The same issue affects Line 147. Either rename the class in D_IMU.h to IMUDriver, or update both references here to MPU6050Driver.

🧰 Tools
🪛 GitHub Actions: PlatformIO CI

[error] 109-109: 'IMUDriver' was not declared in this scope; did you mean 'IRDriver'?

[error] 109-109: no matching function for call to 'getNameAndTags()' (template argument deduction/substitution failed)

🤖 Prompt for AI Agents 
In `@src/MoonLight/Modules/ModuleDrivers.h` at line 109, The build fails because
the code calls getNameAndTags<IMUDriver>() in addControlValue but the actual
class in D_IMU.h is defined as MPU6050Driver; update the template type
references to the real class name (replace IMUDriver with MPU6050Driver)
wherever getNameAndTags<IMUDriver>() and related uses appear (e.g., the
addControlValue(control, getNameAndTags<...>()) calls around the IMU driver
registrations), or alternatively rename the class in D_IMU.h to IMUDriver—ensure
consistency between the class declaration (MPU6050Driver) and all template/type
usages (getNameAndTags and addControlValue).

addControlValue(control, getNameAndTags<HUB75Driver>());

// board preset specific
Expand Down Expand Up @@ -143,6 +144,7 @@ class ModuleDrivers : public NodeManager {
if (!node) node = checkAndAlloc<ArtNetOutDriver>(name);
if (!node) node = checkAndAlloc<AudioSyncDriver>(name);
if (!node) node = checkAndAlloc<IRDriver>(name);
if (!node) node = checkAndAlloc<IMUDriver>(name);
if (!node) node = checkAndAlloc<HUB75Driver>(name);

// board preset specific
Expand Down
168 changes: 168 additions & 0 deletions src/MoonLight/Nodes/Drivers/D_IMU.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/**
@title MoonLight
@file D_MPU6050.h
@repo https://github.com/MoonModules/MoonLight, submit changes to this file as PRs
@Authors https://github.com/MoonModules/MoonLight/commits/main
@Doc https://moonmodules.org/MoonLight/moonlight/overview/
@Copyright © 2026 Github MoonLight Commit Authors
@license GNU GENERAL PUBLIC LICENSE Version 3, 29 June 2007
@license For non GPL-v3 usage, commercial licenses must be purchased. Contact us for more information.
**/

#if FT_MOONLIGHT

// #include <BMI160Gen.h>
#include <MPU6050_6Axis_MotionApps20.h>

class IMUDriver : public Node {
public:
static const char* name() { return "IMU driver"; } // Inertial Measurement Unit
static uint8_t dim() { return _NoD; }
static const char* tags() { return "☸️"; }

bool motionTrackingReady = false; // set true if DMP init was successful

Coord3D gyro; // in degrees (not radians)
Coord3D accell;
uint8_t board = 0;

void setup() override {
addControl(gyro, "gyro", "coord3D");
addControl(accell, "accell", "coord3D");
// isEnabled = false; // need to enable after fresh setup
addControl(board, "board", "select");
addControlValue("MPU6050");
addControlValue("BMI160"); // not supported yet
Comment on lines +29 to +35
Copy link

@coderabbitai coderabbitai bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

board selection has no runtime guard — switching board type while running could cause issues.

If a user changes board from 0 (MPU6050) to 1 (BMI160) at runtime without re-triggering onUpdate, loop20ms will enter the BMI160 branch (which is a no-op) while motionTrackingReady is still true from the MPU6050 init. Consider resetting motionTrackingReady when board changes, or at minimum documenting that a re-enable is required.

🤖 Prompt for AI Agents 
In `@src/MoonLight/Nodes/Drivers/D_IMU.h` around lines 29 - 35, The board
selection control (added in setup via addControl(board,...)) can flip at runtime
without clearing state, so update handling must reset motion-tracking state; add
a guard in onUpdate (or a change callback for the "board" control) that detects
when board changes and sets motionTrackingReady = false and
disables/re-initializes motion tracking (e.g., set isEnabled = false or call the
MPU6050 teardown/init sequence) so loop20ms's BMI160 branch cannot run while
MPU6050 state remains valid; reference functions/vars: setup,
addControl(board,...), board, onUpdate, loop20ms, motionTrackingReady, isEnabled
(or the MPU init/teardown helpers) to locate where to implement this reset.

}

void onUpdate(const Char<20>& oldValue, const JsonObject& control) override {
// add your custom onUpdate code here
if (!control["on"].isNull()) { // control is the node n case of on!
if (control["on"] == true) {
bool i2cInited = true; // todo: check in moduleIO if successfull
if (i2cInited) {
if (board == 0) { // MPU6050
mpu.initialize();

// delay(100);

// verify connection
if (mpu.testConnection()) {
EXT_LOGI(ML_TAG, "MPU6050 connection successful Initializing DMP...");
uint8_t devStatus = mpu.dmpInitialize();

if (devStatus == 0) {
// // Calibration Time: generate offsets and calibrate our MPU6050
mpu.CalibrateAccel(6);
mpu.CalibrateGyro(6);
// mpu.PrintActiveOffsets();

mpu.setDMPEnabled(true); // mandatory

// mpuIntStatus = mpu.getIntStatus();

motionTrackingReady = true;
} else {
// ERROR!
// 1 = initial memory load failed
// 2 = DMP configuration updates failed
// (if it's going to break, usually the code will be 1)
EXT_LOGW(ML_TAG, "DMP Initialization failed (code %d)", devStatus);
}
} else
EXT_LOGW(ML_TAG, "Testing device connections MPU6050 connection failed");
} else if (board == 1) { // BMI160 - NEW

// BMI160.begin(BMI160GenClass::I2C_MODE, 0x68);

// if (BMI160.getDeviceID() == 0xD1) { // BMI160 device ID
// EXT_LOGI(ML_TAG, "BMI160 connection successful");
// motionTrackingReady = true;
// } else {
// EXT_LOGW(ML_TAG, "BMI160 connection failed");
// }
}
}
}
}
}
Comment on lines +38 to +88
Copy link

@coderabbitai coderabbitai bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

No cleanup path when the driver is turned off — DMP left running, motionTrackingReady never reset.

When control["on"] == false (or when the node is destroyed), the DMP remains enabled and motionTrackingReady stays true, so loop20ms keeps reading. Add an else branch to disable DMP and reset state, and mirror that in the destructor.

Proposed fix sketch 
       if (control["on"] == true) {
         // ... existing init code ...
+      } else {
+        if (motionTrackingReady) {
+          mpu.setDMPEnabled(false);
+          motionTrackingReady = false;
+        }
       }

And in the destructor:

-  ~IMUDriver() override {};
+  ~IMUDriver() override {
+    if (motionTrackingReady) {
+      mpu.setDMPEnabled(false);
+      motionTrackingReady = false;
+    }
+  };
🤖 Prompt for AI Agents 
In `@src/MoonLight/Nodes/Drivers/D_IMU.h` around lines 38 - 88, Add a cleanup path
for when the driver is turned off by modifying onUpdate in D_IMU: when
control["on"] == false (or control["on"] is present and false) call the MPU
shutdown steps—disable the DMP via mpu.setDMPEnabled(false) (or equivalent),
clear any interrupt/status flags, stop any periodic reads, and set
motionTrackingReady = false; also ensure any board==1 BMI160 cleanup is
mirrored. Then implement the same teardown in the D_IMU destructor (~D_IMU) to
disable DMP/interrupts and reset motionTrackingReady so loop20ms stops reading
after node removal. Ensure you reference the existing symbols onUpdate,
motionTrackingReady, mpu.setDMPEnabled (and ~D_IMU) when making changes.


void loop20ms() override {
// mpu.getMotion6(&accell.x, &accell.y, &accell.z, &gyro.x, &gyro.y, &gyro.z);
// // display tab-separated accel/gyro x/y/z values
// EXT_LOGI(ML_TAG, "mpu6050 %d,%d,%d %d,%d,%d", accell.x, accell.y, accell.z, gyro.x, gyro.y, gyro.z);

// if programming failed, don't try to do anything
if (!motionTrackingReady) return;
// read a packet from FIFO
if (board == 0) { // MPU6050
if (mpu.dmpGetCurrentFIFOPacket(fifoBuffer)) { // Get the Latest packet
mpu.dmpGetQuaternion(&q, fifoBuffer);
mpu.dmpGetGravity(&gravity, &q);
mpu.dmpGetYawPitchRoll(ypr, &q, &gravity);
gyro.y = ypr[0] * 180 / M_PI; // pan = yaw !
gyro.x = ypr[1] * 180 / M_PI; // tilt = pitch !
gyro.z = ypr[2] * 180 / M_PI; // roll = roll
sharedData.gravity.x = gravity.x * INT16_MAX;
sharedData.gravity.y = gravity.y * INT16_MAX;
sharedData.gravity.z = gravity.z * INT16_MAX;
// display real acceleration, adjusted to remove gravity

EXT_LOGD(ML_TAG, "%f %f %f", gravity.x, gravity.y, gravity.z);

// needed to repeat the following 3 lines (yes if you look at the output: otherwise not 0)
// mpu.dmpGetQuaternion(&q, fifoBuffer);
mpu.dmpGetAccel(&aa, fifoBuffer);
// mpu.dmpGetGravity(&gravity, &q);

mpu.dmpGetLinearAccel(&aaReal, &aa, &gravity);
// mpu.dmpGetLinearAccelInWorld(&aaWorld, &aaReal, &q); //worked in 0.6.0, not in 1.3.0 anymore

accell.x = aaReal.x;
accell.y = aaReal.y;
accell.z = aaReal.z;
}
} else if (board == 1) { // BMI160 - NEW
// int gx, gy, gz, ax, ay, az;
// BMI160.readGyro(gx, gy, gz);
// BMI160.readAccelerometer(ax, ay, az);

// // Convert raw values to degrees (BMI160 gyro: 16.4 LSB/°/s at ±2000°/s range)
// gyro.x = gx / 16.4f;
// gyro.y = gy / 16.4f;
// gyro.z = gz / 16.4f;

// // Convert raw accel values (BMI160 accel: 16384 LSB/g at ±2g range)
// accell.x = ax;
// accell.y = ay;
// accell.z = az;

// // Calculate gravity vector from accelerometer
// float norm = sqrt(ax*ax + ay*ay + az*az);
// if (norm > 0) {
// sharedData.gravity.x = (ax / norm) * INT16_MAX;
// sharedData.gravity.y = (ay / norm) * INT16_MAX;
// sharedData.gravity.z = (az / norm) * INT16_MAX;
// }
}
};

~IMUDriver() override {};

private:
MPU6050 mpu;

// MPU control/status vars
uint8_t fifoBuffer[64]; // FIFO storage buffer

// orientation/motion vars
Quaternion q; // [w, x, y, z] quaternion container
VectorInt16 aa; // [x, y, z] accel sensor measurements
VectorInt16 aaReal; // [x, y, z] gravity-free accel sensor measurements
// VectorInt16 aaWorld; // [x, y, z] world-frame accel sensor measurements
VectorFloat gravity; // [x, y, z] gravity vector
// float euler[3]; // [psi, theta, phi] Euler angle container
float ypr[3]; // [yaw, pitch, roll] yaw/pitch/roll container and gravity vector
};

#endif
2 changes: 1 addition & 1 deletion src/MoonLight/Nodes/Drivers/D__Sandbox.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ExampleDriver : public Node {
addControl(pin, "pin", "slider", 1, SOC_GPIO_PIN_COUNT - 1);
}

void onUpdate(const Char<20>& oldValue, const JsonObject& control) {
void onUpdate(const Char<20>& oldValue, const JsonObject& control) override {
// add your custom onUpdate code here
if (control["name"] == "pin") {
if (control["value"] == 0) {
Expand Down
Loading