Add support for bq2589x battery charger (Usermod v2) #4892
Conversation
Summary: Added new usermod v2: usermod_v2_bq2589x for supporting bq2589x family chips (BQ25890/BQ25892/BQ25895). Uses the memeexpert/bq2589x library (added as a dependency). Implements battery status display: voltage, current, temperature, charge level, and power source in WLED JSON API and web interface. Support for configuring pins, polling interval, max charge current, and temperature via usermod settings. Added methods for reading battery parameters and controlling OTG. Added documentation (readme.md) with installation and usage instructions. Changes: New module: usermod_v2_bq2589x Added library.json with memeexpert/bq2589x dependency. Updated constants and PinManager for new usermod support. Added configuration template and usage examples. Testing: Verified build and operation on ESP32-C3. Web interface displays battery parameters. Settings are correctly applied via Usermod Settings. Documentation and configuration examples are available in readme.md.
WalkthroughAdds a new WLED v2 usermod for the TI BQ2589x charger including implementation, PlatformIO library manifest, and README; registers a new USERMOD_ID and PinOwner entry for core pin management. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
usermods/usermod_v2_bq2589x/readme.md (1)
1-76: Fix user-facing grammar/typos and keep docs consistent with code.
- Correct several grammatical issues and “custom_usermds” → “custom_usermods”.
- The README claims “change battery max/min voltages”, but the code doesn’t expose these settings. Please remove or add the missing config fields to the usermod.
Apply:
-# bq2589x - -The BQ25895 is a highly-integrated 5-A switch-mode battery charge management -and system power path management device for single cell Li-Ion and Li-polymer -battery. The devices support high input voltage fast charging. +# bq2589x + +The BQ25895 is a highly integrated 5 A switch‑mode battery‑charge and system power‑path +management device for a single‑cell Li‑ion/Li‑polymer battery. The device supports +high input‑voltage fast charging. @@ [env:esp32bq2589x] extends = env:esp32c3dev -custom_usermods = usermod_v2_bq2589x # add "custom_usermds" to the extensible environment if it is not there +custom_usermods = usermod_v2_bq2589x # add "custom_usermods" to the extensible environment if it is not there @@ -You can change default parameters by add build flags like this: +You can change default parameters by adding build flags like this: @@ --D BQ2589X_SYS_PIN=1 # input pin for reading sys voltage by ADC of your controller. +-D BQ2589X_SYS_PIN=1 # input pin for reading system voltage via the controller ADC @@ --DWLED_DEBUG # for enable debug print +-DWLED_DEBUG # enable debug prints @@ -After compiling and upload your code you can see some params from your battery -by click "info" button in UI. -You can change battery max/min voltages, request frequency, pins and charger -charger current in usermods config. +After compiling and uploading, you can see battery parameters by clicking the “Info” +button in the UI. +You can change the polling interval, pins, maximum charge current, and maximum +temperature in the Usermods settings. @@ -Be sure to use a capacitor on the OTG line and the host controller power supply -to avoid "blinking" and voltage drops in the host controller supply. +Use a capacitor on the OTG line and on the host‑controller power rail to avoid flicker +and voltage droops. @@ -- Add more config features (like another battery type, precharge current control and etc); +- Add more config features (e.g., other battery profiles, precharge current control, etc.);
🧹 Nitpick comments (8)
wled00/const.h (1)
202-203: Usermod ID added; please confirm uniqueness and future-proofing.59 fits the sequence, but please verify no collisions with other open PRs and consider that numeric IDs may be removed in the future. If minimizing core churn is preferred, an alternative is hard-coding the ID inside the usermod (as some mods do) and mapping PinOwner via a compile-time flag. Your current approach is fine if maintainers prefer centralized IDs.
usermods/usermod_v2_bq2589x/library.json (1)
1-7: Align dependency spec with README and decide update policy.README uses caret (^1.0.3) while this manifest uses tilde (~1.0.3). Pick one:
- Caret (^) to allow minor+patch updates, or
- Tilde (~) to allow patch-only updates.
Recommend caret for broader compatibility unless you need to pin minor.
Apply:
- "memeexpert/bq2589x":"~1.0.3" + "memeexpert/bq2589x":"^1.0.3"usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp (6)
378-395: Avoid hijacking global “on” state; scope OTG control under your usermod.Reading root["on"] couples OTG with WLED’s main power. Prefer a namespaced key, e.g., root["bq2589x"]["otg"].
Apply:
- JsonObject usermod = root[FPSTR(_name)]; - - if(root["on"].is<bool>()) { - otg_status = root["on"].as<bool>(); - } else if (root["on"].is<const char*>()) { - if(root["on"].as<const char*>()[0] == 't') otg_status = true; - else otg_status = false; - } + JsonObject usermod = root[FPSTR(_name)]; + if (!usermod.isNull()) { + if (usermod["otg"].is<bool>()) { + otg_status = usermod["otg"].as<bool>(); + } else if (usermod["otg"].is<const char*>()) { + otg_status = (usermod["otg"].as<const char*>()[0] == 't'); + } + }
137-148: Typo and minor labels.
- “CDP(1,5A)” and “DCP(3,25A)” should use periods, not commas.
Apply:
- case 2: s = "CDP(1,5A)"; break; + case 2: s = "CDP(1.5A)"; break; - case 3: s = "DCP(3,25A)"; break; + case 3: s = "DCP(3.25A)"; break;
201-209: Avoid reinitializing Serial; WLED manages it.Calling Serial.begin() here can conflict with global logging settings. Safe to remove.
Apply:
- Serial.begin(115200); Wire.begin();
618-630: Variable misnamed in getChargerCurrent().The local variable is named “temperature”; name it “current” to match semantics.
Apply:
- int getChargerCurrent() { - mycharger.adc_start(false); - int temperature = mycharger.adc_read_charge_current(); - mycharger.adc_stop(); - return temperature; - } + int getChargerCurrent() { + mycharger.adc_start(false); + int current = mycharger.adc_read_charge_current(); + mycharger.adc_stop(); + return current; + }
69-73: Remove unused PROGMEM keys or implement features._batMinV/_batMaxV are declared but unused; either remove or wire them into config and UI.
Apply:
- static const char _batMinV[]; - static const char _batMaxV[];
675-677: Typo in config key: “!ce-pin”.Likely intended “nce-pin”.
Apply:
-const char Usermod_v2_bq2589x::_ncePin[] PROGMEM = "!ce-pin"; +const char Usermod_v2_bq2589x::_ncePin[] PROGMEM = "nce-pin";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
usermods/usermod_v2_bq2589x/library.json(1 hunks)usermods/usermod_v2_bq2589x/readme.md(1 hunks)usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp(1 hunks)wled00/const.h(1 hunks)wled00/pin_manager.h(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/{*.cpp,!(html_*)*.h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ source and header files
Files:
wled00/const.hwled00/pin_manager.h
🧠 Learnings (2)
📚 Learning: 2025-03-29T01:22:54.617Z
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Applied to files:
wled00/const.hwled00/pin_manager.h
📚 Learning: 2025-04-18T22:27:58.634Z
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
Applied to files:
usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp
🧬 Code graph analysis (2)
wled00/const.h (1)
usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp (1)
USERMOD_ID_BQ2589X(595-598)
wled00/pin_manager.h (2)
usermods/pixels_dice_tray/pixels_dice_tray.cpp (1)
USERMOD_ID_PIXELS_DICE_TRAY(528-528)usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp (1)
USERMOD_ID_BQ2589X(595-598)
🪛 LanguageTool
usermods/usermod_v2_bq2589x/readme.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...-A switch-mode battery charge management and system power path management device ...
(QB_NEW_EN)
[grammar] ~4-~4: There might be a mistake here.
Context: ...ce for single cell Li-Ion and Li-polymer battery. The devices support high input voltage...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...ou can see some params from your battery by click "info" button in UI. You can ...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...rom your battery by click "info" button in UI. You can change battery max/min vo...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...ur battery by click "info" button in UI. You can change battery max/min voltages,...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...ges, request frequency, pins and charger charger current in usermods config. ## Tips ...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...ine and the host controller power supply to avoid "blinking" and voltage drops in...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...ke to jointly improve the code. There is room for development of this usermod. ...
(QB_NEW_EN)
[style] ~75-~75: In American English, abbreviations like “etc.” require a period.
Context: ...ery type, precharge current control and etc);
(ETC_PERIOD)
[grammar] ~75-~75: There might be a mistake here.
Context: ...ype, precharge current control and etc);
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
usermods/usermod_v2_bq2589x/readme.md
65-65: Bare URL used
(MD034, no-bare-urls)
65-65: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
wled00/pin_manager.h (1)
71-73: PinOwner added for BQ2589X — LGTM.Enum entry and trailing comma on the previous line look correct.
| void loop() override { | ||
| static bool timerOn = true; | ||
| // if usermod is disabled or called during strip updating just exit | ||
| // NOTE: on very long strips strip.isUpdating() may always return true so update accordingly | ||
| if (!enabled || strip.isUpdating()) return; | ||
| // do your magic here | ||
| if (millis() - lastTime > requestInterval) { | ||
| if (configChanged()) { | ||
| DEBUG_PRINTLN(F("bq2589x usermod config changed, sending to bq2589x")); | ||
| timerOn = false; | ||
| sendUserConfigToBq(); | ||
| } else if (!timerOn) { | ||
| DEBUG_PRINTLN(F("bq2589x config set to default, enabling watchdog timer")); | ||
| mycharger.reset_chip(); | ||
| timerOn = true; | ||
| } | ||
| } | ||
| lastTime = millis(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Timer logic bug: lastTime is reset every loop iteration.
Moving lastTime update inside the timed block fixes the interval logic.
Apply:
- if (millis() - lastTime > requestInterval) {
+ if (millis() - lastTime >= requestInterval) {
if (configChanged()) {
DEBUG_PRINTLN(F("bq2589x usermod config changed, sending to bq2589x"));
timerOn = false;
sendUserConfigToBq();
} else if (!timerOn) {
DEBUG_PRINTLN(F("bq2589x config set to default, enabling watchdog timer"));
mycharger.reset_chip();
timerOn = true;
}
- }
- lastTime = millis();
- }
+ lastTime = millis();
+ }
+ }📝 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.
| void loop() override { | |
| static bool timerOn = true; | |
| // if usermod is disabled or called during strip updating just exit | |
| // NOTE: on very long strips strip.isUpdating() may always return true so update accordingly | |
| if (!enabled || strip.isUpdating()) return; | |
| // do your magic here | |
| if (millis() - lastTime > requestInterval) { | |
| if (configChanged()) { | |
| DEBUG_PRINTLN(F("bq2589x usermod config changed, sending to bq2589x")); | |
| timerOn = false; | |
| sendUserConfigToBq(); | |
| } else if (!timerOn) { | |
| DEBUG_PRINTLN(F("bq2589x config set to default, enabling watchdog timer")); | |
| mycharger.reset_chip(); | |
| timerOn = true; | |
| } | |
| } | |
| lastTime = millis(); | |
| } | |
| void loop() override { | |
| static bool timerOn = true; | |
| // if usermod is disabled or called during strip updating just exit | |
| // NOTE: on very long strips strip.isUpdating() may always return true so update accordingly | |
| if (!enabled || strip.isUpdating()) return; | |
| // do your magic here | |
| if (millis() - lastTime >= requestInterval) { | |
| if (configChanged()) { | |
| DEBUG_PRINTLN(F("bq2589x usermod config changed, sending to bq2589x")); | |
| timerOn = false; | |
| sendUserConfigToBq(); | |
| } else if (!timerOn) { | |
| DEBUG_PRINTLN(F("bq2589x config set to default, enabling watchdog timer")); | |
| mycharger.reset_chip(); | |
| timerOn = true; | |
| } | |
| lastTime = millis(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp around lines 273 to 292,
the timer logic resets lastTime on every loop iteration which breaks the
interval check; move the lastTime = millis() assignment inside the if (millis()
- lastTime > requestInterval) block (after handling configChanged()/watchdog
logic) so lastTime is only updated when the timed action runs, preserving
correct interval behavior.
| bool readFromConfig(JsonObject& root) override | ||
| { | ||
| // default settings values could be set here (or below using the 3-argument getJsonValue()) instead of in the class definition or constructor | ||
| // setting them inside readFromConfig() is slightly more robust, handling the rare but plausible use case of single value being missing after boot (e.g. if the cfg.json was manually edited and a value was removed) | ||
| JsonObject top = root[FPSTR(_name)]; | ||
| if (top.isNull()) { | ||
| DEBUG_PRINTF("[%s] No config found. (Using defaults.)\n", _name); | ||
| return false; | ||
| } | ||
| uint8_t oldPinInt = int_pin; | ||
| uint8_t oldPinNce = nce_pin; | ||
| uint8_t oldPinOtg = otg_pin; | ||
| uint8_t oldPinSys = sys_pin; | ||
| bool oldEnabled = enabled; | ||
|
|
||
| getJsonValue(top[FPSTR(_otgPin)], otg_pin); | ||
| getJsonValue(top[FPSTR(_ncePin)], nce_pin); | ||
| getJsonValue(top[FPSTR(_intPin)], int_pin); | ||
| getJsonValue(top[FPSTR(_sysPin)], sys_pin); | ||
| getJsonValue(top[FPSTR(_batMaxCC)], bat_max_charge_current); | ||
| getJsonValue(top[FPSTR(_requestInterval)], requestInterval); | ||
| getJsonValue(top[FPSTR(_enabled)], enabled); | ||
| getJsonValue(top[FPSTR(_maxTemperature)], bat_temperature_max); | ||
|
|
||
| if (enabled != oldEnabled) { | ||
| deinitPins(); | ||
| DEBUG_PRINTF("bq2589 %s\n", enabled ? "enabled" : "disabled"); | ||
| } | ||
|
|
||
| if (enabled && ( | ||
| oldPinInt != int_pin || | ||
| oldPinNce != nce_pin || | ||
| oldPinOtg != otg_pin || | ||
| oldPinSys != sys_pin) | ||
| ) { | ||
| DEBUG_PRINTF("bq2589x reinit pins\n"); | ||
| reinitPins(); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Enabling the usermod doesn’t re-initialize pins.
When enabled flips from false→true, initPins() is never called.
Apply:
- if (enabled != oldEnabled) {
- deinitPins();
- DEBUG_PRINTF("bq2589 %s\n", enabled ? "enabled" : "disabled");
- }
+ if (enabled != oldEnabled) {
+ if (enabled) initPins(); else deinitPins();
+ DEBUG_PRINTF("bq2589x %s\n", enabled ? "enabled" : "disabled");
+ }📝 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.
| bool readFromConfig(JsonObject& root) override | |
| { | |
| // default settings values could be set here (or below using the 3-argument getJsonValue()) instead of in the class definition or constructor | |
| // setting them inside readFromConfig() is slightly more robust, handling the rare but plausible use case of single value being missing after boot (e.g. if the cfg.json was manually edited and a value was removed) | |
| JsonObject top = root[FPSTR(_name)]; | |
| if (top.isNull()) { | |
| DEBUG_PRINTF("[%s] No config found. (Using defaults.)\n", _name); | |
| return false; | |
| } | |
| uint8_t oldPinInt = int_pin; | |
| uint8_t oldPinNce = nce_pin; | |
| uint8_t oldPinOtg = otg_pin; | |
| uint8_t oldPinSys = sys_pin; | |
| bool oldEnabled = enabled; | |
| getJsonValue(top[FPSTR(_otgPin)], otg_pin); | |
| getJsonValue(top[FPSTR(_ncePin)], nce_pin); | |
| getJsonValue(top[FPSTR(_intPin)], int_pin); | |
| getJsonValue(top[FPSTR(_sysPin)], sys_pin); | |
| getJsonValue(top[FPSTR(_batMaxCC)], bat_max_charge_current); | |
| getJsonValue(top[FPSTR(_requestInterval)], requestInterval); | |
| getJsonValue(top[FPSTR(_enabled)], enabled); | |
| getJsonValue(top[FPSTR(_maxTemperature)], bat_temperature_max); | |
| if (enabled != oldEnabled) { | |
| deinitPins(); | |
| DEBUG_PRINTF("bq2589 %s\n", enabled ? "enabled" : "disabled"); | |
| } | |
| if (enabled && ( | |
| oldPinInt != int_pin || | |
| oldPinNce != nce_pin || | |
| oldPinOtg != otg_pin || | |
| oldPinSys != sys_pin) | |
| ) { | |
| DEBUG_PRINTF("bq2589x reinit pins\n"); | |
| reinitPins(); | |
| } | |
| return true; | |
| } | |
| bool readFromConfig(JsonObject& root) override | |
| { | |
| // default settings values could be set here (or below using the 3-argument getJsonValue()) instead of in the class definition or constructor | |
| // setting them inside readFromConfig() is slightly more robust, handling the rare but plausible use case of single value being missing after boot (e.g. if the cfg.json was manually edited and a value was removed) | |
| JsonObject top = root[FPSTR(_name)]; | |
| if (top.isNull()) { | |
| DEBUG_PRINTF("[%s] No config found. (Using defaults.)\n", _name); | |
| return false; | |
| } | |
| uint8_t oldPinInt = int_pin; | |
| uint8_t oldPinNce = nce_pin; | |
| uint8_t oldPinOtg = otg_pin; | |
| uint8_t oldPinSys = sys_pin; | |
| bool oldEnabled = enabled; | |
| getJsonValue(top[FPSTR(_otgPin)], otg_pin); | |
| getJsonValue(top[FPSTR(_ncePin)], nce_pin); | |
| getJsonValue(top[FPSTR(_intPin)], int_pin); | |
| getJsonValue(top[FPSTR(_sysPin)], sys_pin); | |
| getJsonValue(top[FPSTR(_batMaxCC)], bat_max_charge_current); | |
| getJsonValue(top[FPSTR(_requestInterval)], requestInterval); | |
| getJsonValue(top[FPSTR(_enabled)], enabled); | |
| getJsonValue(top[FPSTR(_maxTemperature)], bat_temperature_max); | |
| if (enabled != oldEnabled) { | |
| if (enabled) initPins(); else deinitPins(); | |
| DEBUG_PRINTF("bq2589x %s\n", enabled ? "enabled" : "disabled"); | |
| } | |
| if (enabled && ( | |
| oldPinInt != int_pin || | |
| oldPinNce != nce_pin || | |
| oldPinOtg != otg_pin || | |
| oldPinSys != sys_pin) | |
| ) { | |
| DEBUG_PRINTF("bq2589x reinit pins\n"); | |
| reinitPins(); | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp around lines 462–502, the
code only calls deinitPins() when enabled changes but never calls initPins()
when enabled flips from false→true; modify the enabled-change branch so that if
enabled changed and is now true you call initPins() (and log "bq2589 enabled"),
and if it changed and is now false you call deinitPins() (and log "bq2589
disabled"); keep the separate pin-change check (reinitPins()) for when pins
changed while already enabled, or alternatively skip reinitPins() and call
initPins() when enabling even if pins changed—ensure initialization happens
whenever enabled becomes true.
| extends = env:esp32c3dev | ||
| custom_usermods = usermod_v2_bq2589x # add "custom_usermds" to the extensible environment if it is not there | ||
| build_flags = ${common.build_flags_esp32} | ||
| -DUSERMOD_V2_BQ2589X |
There was a problem hiding this comment.
Do you actually use USERMOD_V2_BQ2589X flag anymore, the new usermod system should make that redundant in favor of the custom_usermods
wled00/pin_manager.h
Outdated
| UM_BME68X = USERMOD_ID_BME68X, // 0x31 // Usermod "usermod_bme68x.h -- Uses "standard" HW_I2C pins | ||
| UM_PIXELS_DICE_TRAY = USERMOD_ID_PIXELS_DICE_TRAY // 0x35 // Usermod "pixels_dice_tray.h" -- Needs compile time specified 6 pins for display including SPI. | ||
| UM_PIXELS_DICE_TRAY = USERMOD_ID_PIXELS_DICE_TRAY, // 0x35 // Usermod "pixels_dice_tray.h" -- Needs compile time specified 6 pins for display including SPI. | ||
| UM_BQ2589X = USERMOD_ID_BQ2589X |
There was a problem hiding this comment.
include the comment, as per the other entries
There was a problem hiding this comment.
I'm sorry, my bad. Just forgot about it.
| static void IRAM_ATTR intPinUp() { | ||
| // just a dummy for now | ||
| } |
There was a problem hiding this comment.
Not sure about your usermod adding a global function like this
There was a problem hiding this comment.
I apologize, I do not quite understand what you are talking about. If i can not register my interruptions, I can remove it. But I saw in usermod_v2_rotary_encoder_ui_alt registration of interruption. If you are about the fact that I do not use it, then I just wanted to clearly indicate to the user that he can add some logic there if he need it. If you think this is unnecessary, I will remove that.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
usermods/usermod_v2_bq2589x/readme.md (1)
15-21: Good: switched to custom_usermods (no -D flag).Using custom_usermods is the right path with v2 usermods and makes the prior USERMOD_V2_BQ2589X flag unnecessary. Also fix a small typo in the comment.
-custom_usermods = usermod_v2_bq2589x # add "custom_usermds" to the extensible environment if it is not there +custom_usermods = usermod_v2_bq2589x # add "custom_usermods" to the extensible environment if it is not there
🧹 Nitpick comments (5)
usermods/usermod_v2_bq2589x/readme.md (5)
20-20: Align dependency version spec with library.json.README shows ^1.0.3 while library.json uses ~1.0.3 per PR notes. Please pick one strategy and keep both places consistent.
26-39: Compilation section: clarify how to add build flags + grammar fixes.Make it copy/paste‑able and consistent (-D spacing, comments), and fix “by add”.
-You can change default parameters by add build flags like this: +You can change default parameters by adding build flags like this: ```ini --D BQ2589X_OTG_PIN=12 # for change OTG default pin --D BQ2589X_NCE_PIN=4 # disable charger --D BQ2589X_INT_PIN=2 # input pin for interrupt by bq --D BQ2589X_SYS_PIN=1 # input pin for reading sys voltage by ADC of your controller. --D BQ2589X_DEFAULT_REQUEST_INTERVAL=5000 # changing request interval --D BQ2589X_DEFAULT_BAT_MAX_CC=2100 # changing max charger current (main charge stage) --D BQ2589X_MAX_TEMPERATURE=80 # change maximal temperatures --DWLED_DEBUG # for enable debug print +; Either append to existing build_flags: +build_flags = + ${common.build_flags_esp32} + -D BQ2589X_OTG_PIN=12 ; change OTG default pin + -D BQ2589X_NCE_PIN=4 ; disable charger + -D BQ2589X_INT_PIN=2 ; interrupt pin from BQ + -D BQ2589X_SYS_PIN=1 ; read system voltage via the MCU ADC + -D BQ2589X_DEFAULT_REQUEST_INTERVAL=5000 ; telemetry polling interval (ms) + -D BQ2589X_DEFAULT_BAT_MAX_CC=2100 ; max charge current (mA) in fast‑charge + -D BQ2589X_MAX_TEMPERATURE=80 ; max temperature (°C) + -D WLED_DEBUG ; enable debug printsAdditionally, consider a short note to choose MCU‑safe GPIOs (strap/ADC‑capable) when overriding pins. --- `51-53`: **Tip: add concrete capacitor guidance.** Suggest typical values/placement to make this actionable. ```diff -Be sure to use a capacitor on the OTG line and the host controller power supply -to avoid "blinking" and voltage drops in the host controller supply. +Place a low‑ESR bulk capacitor (e.g., 100–470 µF) close to the OTG pin/load and on the host controller 5 V rail +to avoid “blinking” and voltage dips under transient load.
63-67: Prefer Issues/Discussions over personal emails; minor grammar tweak.Keeps support public/searchable and avoids publishing PII in repos.
-Maintainer: [memeexpert](https://github.com/memExpert). -Write me an email ([email protected] or [email protected]) if you -found a bug or inaccuracy or would like to jointly improve the code. There is -room for development of this usermod. +Maintainer: [memeexpert](https://github.com/memExpert). +Please open a GitHub Issue if you find a bug or inaccuracy, or to propose improvements. +There is room for development of this usermod.
71-75: List: fix punctuation/“etc.” usage.-- Add MQTT support; -- Add more config features (like another battery type, precharge current control and etc); +- Add MQTT support. +- Add more config features (e.g., additional battery types, precharge current control, etc.).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
usermods/usermod_v2_bq2589x/readme.md(1 hunks)usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp(1 hunks)wled00/pin_manager.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- wled00/pin_manager.h
- usermods/usermod_v2_bq2589x/usermod_v2_bq2589x.cpp
🧰 Additional context used
🪛 LanguageTool
usermods/usermod_v2_bq2589x/readme.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...-A switch-mode battery charge management and system power path management device ...
(QB_NEW_EN)
[grammar] ~4-~4: There might be a mistake here.
Context: ...ce for single cell Li-Ion and Li-polymer battery. The devices support high input voltage...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...ou can see some params from your battery by click "info" button in UI. You can ...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...rom your battery by click "info" button in UI. You can change battery max/min vo...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...ur battery by click "info" button in UI. You can change battery max/min voltages,...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...ges, request frequency, pins and charger charger current in usermods config. ## Tips ...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...ine and the host controller power supply to avoid "blinking" and voltage drops in...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...ke to jointly improve the code. There is room for development of this usermod. ...
(QB_NEW_EN)
[style] ~74-~74: In American English, abbreviations like “etc.” require a period.
Context: ...ery type, precharge current control and etc);
(ETC_PERIOD)
[grammar] ~74-~74: There might be a mistake here.
Context: ...ype, precharge current control and etc);
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
usermods/usermod_v2_bq2589x/readme.md
64-64: Bare URL used
(MD034, no-bare-urls)
64-64: Bare URL used
(MD034, no-bare-urls)
| The BQ25895 is a highly-integrated 5-A switch-mode battery charge management | ||
| and system power path management device for single cell Li-Ion and Li-polymer | ||
| battery. The devices support high input voltage fast charging. | ||
|
|
There was a problem hiding this comment.
Polish intro: fix grammar and scope to the whole BQ2589x family.
Tighten wording and correct casing/plurals.
Apply:
-The BQ25895 is a highly-integrated 5-A switch-mode battery charge management
-and system power path management device for single cell Li-Ion and Li-polymer
-battery. The devices support high input voltage fast charging.
+TI BQ2589x (BQ25890/BQ25892/BQ25895) is a highly integrated 5‑A switch‑mode
+battery charger and system power‑path management IC for single‑cell
+Li‑ion/Li‑polymer batteries. It supports high‑input‑voltage fast charging.📝 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.
| The BQ25895 is a highly-integrated 5-A switch-mode battery charge management | |
| and system power path management device for single cell Li-Ion and Li-polymer | |
| battery. The devices support high input voltage fast charging. | |
| TI BQ2589x (BQ25890/BQ25892/BQ25895) is a highly integrated 5-A switch-mode | |
| battery charger and system power-path management IC for single-cell | |
| Li-ion/Li-polymer batteries. It supports high-input-voltage fast charging. |
🧰 Tools
🪛 LanguageTool
[grammar] ~3-~3: There might be a mistake here.
Context: ...-A switch-mode battery charge management and system power path management device ...
(QB_NEW_EN)
[grammar] ~4-~4: There might be a mistake here.
Context: ...ce for single cell Li-Ion and Li-polymer battery. The devices support high input voltage...
(QB_NEW_EN)
| After compiling and upload your code you can see some params from your battery | ||
| by click "info" button in UI. | ||
| You can change battery max/min voltages, request frequency, pins and charger | ||
| charger current in usermods config. |
There was a problem hiding this comment.
Usage: fix grammar and duplicate word; reference “Usermod Settings”.
-After compiling and upload your code you can see some params from your battery
-by click "info" button in UI.
-You can change battery max/min voltages, request frequency, pins and charger
-charger current in usermods config.
+After compiling and uploading, click the “Info” button in the UI to view battery parameters.
+You can change battery min/max voltages, polling interval, pins, and charge current in Usermod Settings.📝 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.
| After compiling and upload your code you can see some params from your battery | |
| by click "info" button in UI. | |
| You can change battery max/min voltages, request frequency, pins and charger | |
| charger current in usermods config. | |
| After compiling and uploading, click the “Info” button in the UI to view battery parameters. | |
| You can change battery min/max voltages, polling interval, pins, and charge current in Usermod Settings. |
🧰 Tools
🪛 LanguageTool
[grammar] ~43-~43: There might be a mistake here.
Context: ...ou can see some params from your battery by click "info" button in UI. You can ...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...rom your battery by click "info" button in UI. You can change battery max/min vo...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...ur battery by click "info" button in UI. You can change battery max/min voltages,...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...ges, request frequency, pins and charger charger current in usermods config. ## Tips ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In usermods/usermod_v2_bq2589x/readme.md around lines 43 to 46, fix grammar and
a duplicated word and standardize the settings reference: change "After
compiling and upload your code you can see some params from your battery by
click "info" button in UI." to use correct gerunds and capitalization ("After
compiling and uploading your code, you can see battery parameters by clicking
the "Info" button in the UI.") and remove the duplicated "charger" in the next
sentence while replacing "usermods config" with "Usermod Settings" (e.g. "You
can change battery max/min voltages, request frequency, pins and charger current
in Usermod Settings.").
|
thanks for contributing. I have a few questions:
|
I using board from CG: https://aliexpress.com/item/1005005660079407.html?sku_id=12000033925273913
Thanks to your remark, I noticed that the BQ25892 uses a different address (6B instead of 6A). I didn’t check this in the forked library. The rest of the interfaces are the same. I’m attaching a screenshot as confirmation.
|
Summary:
Added new usermod v2: usermod_v2_bq2589x for supporting bq2589x family chips (BQ25890/BQ25892/BQ25895).
Uses the memeexpert/bq2589x library (added as a dependency).
Implements battery status display: voltage, current, temperature, charge level, and power source in WLED JSON API and web interface.
Support for configuring pins, polling interval, max charge current, and temperature via usermod settings.
Added methods for reading battery parameters and controlling OTG.
Added documentation (readme.md) with installation and usage instructions.
Changes:
Testing:
If you find any inaccuracies or want me to add/change any logic, let me know. It seems to me that this usermod will be useful to people who want to use WLED on batteries.
Summary by CodeRabbit
New Features
Documentation
Chores