-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Usermod for LM75 I2C Temperature Sensors #4680
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new usermod for WLED has been introduced to support LM75 I2C temperature sensors, including overtemperature protection with configurable auto-off thresholds. The module handles sensor detection, periodic temperature readings, MQTT integration, Home Assistant autodiscovery, configuration persistence, and exposes configuration options through WLED’s UI and persistent storage. Documentation and a library configuration file have also been added. Changes
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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. 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (14)
usermods/LM75_Temperature/library.json (1)
7-8
: Remove trailing whitespace.There's an extra whitespace line at the end of the file that should be removed for cleaner code.
} -
usermods/LM75_Temperature/readme.md (1)
1-58
: Documentation is comprehensive but contains grammatical issues.The readme provides excellent documentation for the usermod including parameters, compilation instructions, and usage notes. However, there are several grammatical and spelling issues that should be addressed.
Here are some corrections:
- Lines 13, 14: Add a subject to form complete sentences: "It can be between 1 and 100" and "It can be between 0 and 99°C"
- Line 26, 38: "Parameter" is misspelled as "Paremeter"
- Line 26, 38: "false" is misspelled as "flase"
- Line 45: Add a comma after "Hence": "Hence, the maximum Temperature..."
- Line 45: "maximum" is misspelled as "maxium"
- Line 45: Remove "at" in "at capped at"
- Line 49: Add a comma: "...of the WLED output, the overtemperature..."
- Line 49: Fix verb agreement: "...the mod checks the inputs and adjusts the lower value..."
- Line 49: "hysteresis" is misspelled as "hystersis" (multiple instances)
- Line 57: Add a comma: "During power up, the mod tries..."
- Line 57: Replace "can not" with "cannot"
🧰 Tools
🪛 LanguageTool
[style] ~13-~13: To form a complete sentence, be sure to include a subject.
Context: ... which the WLED output is switched off. Can be between 1 and 100 ...(MISSING_IT_THERE)
[style] ~14-~14: To form a complete sentence, be sure to include a subject.
Context: ...mum 1 towards the shutdown temperature. Can be beween 0 and 99°C | 40°C | ## Comp...(MISSING_IT_THERE)
[uncategorized] ~45-~45: A comma may be missing after the conjunctive/linking adverb ‘Hence’.
Context: ...LED only allows uint8 values as inputs. Hence the maxium Temperature at capped at 212...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~49-~49: A comma might be missing here.
Context: ...
To avoid flickering of the WLED output the overtemperature feature has a hyste...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...gger hystersis when setting the limits. Still the mod checks the inputs and adjust th...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~49-~49: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...ts. Still the mod checks the inputs and adjust the lower value not to equal or higher ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~57-~57: Possible missing comma found.
Context: ...s of now untested.
During power up the mod tries to scan for the I2C devic...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~57-~57: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...ge shows an error message if the sensor can not be reached.(CAN_NOT_PREMIUM)
usermods/LM75_Temperature/LM75.cpp (2)
3-3
: Use predefined constants for I2C address.The code uses a hard-coded I2C address for the LM75 sensor, which is defined by a build flag. Consider adding a default value in case the build flag is not defined.
+#ifndef USERMOD_LM75TEMPERATURE_I2C_ADRESS +#define USERMOD_LM75TEMPERATURE_I2C_ADRESS 0x48 // Default LM75 address +#endif + static Generic_LM75 LM75temperature(USERMOD_LM75TEMPERATURE_I2C_ADRESS);
136-136
: Remove debugging message.The "IDKFA" string appears to be a leftover debug message or easter egg (Doom cheat code). This should be removed or replaced with a more descriptive debug message.
- DEBUG_PRINTLN(F("IDKFA")); + DEBUG_PRINTLN(F("Reading temperature..."));usermods/LM75_Temperature/LM75.h (10)
28-29
: Fix the typo in I2C_ADRESS macro name.There's a spelling error in the macro name - "ADRESS" should be "ADDRESS".
-#ifndef USERMOD_LM75TEMPERATURE_I2C_ADRESS - #define USERMOD_LM75TEMPERATURE_I2C_ADRESS 0x48 +#ifndef USERMOD_LM75TEMPERATURE_I2C_ADDRESS + #define USERMOD_LM75TEMPERATURE_I2C_ADDRESS 0x48
45-45
: Simplify timing initialization for first temperature reading.Using
UINT32_MAX - USERMOD_LM75TEMPERATURE_MEASUREMENT_INTERVAL
to set up the initial reading timing is unnecessarily complex and may cause issues with timer overflow.- unsigned long lastMeasurement = UINT32_MAX - USERMOD_LM75TEMPERATURE_MEASUREMENT_INTERVAL; + // Set to trigger first reading 20 seconds after boot + unsigned long lastMeasurement = 0;Then in your setup() function, you can initialize with:
lastMeasurement = millis() - readingInterval + 20000; // First reading after 20 seconds
58-58
: Follow consistent camelCase naming convention.The variable
overtemptriggered
should follow camelCase naming convention.- bool overtemptriggered = false; + bool overtempTriggered = false;
62-62
: Add documentation for HApublished variable.The
HApublished
variable lacks documentation, unlike other member variables in this class.+ // Tracks whether Home Assistant autodiscovery has been published bool HApublished = false;
82-84
: Fix typo in API comment.There are typos in the API comment.
- * API calls te enable data exchan)ge between WLED modules + * API calls to enable data exchange between WLED modules
85-87
: Clarify temperature unit in API method documentation.The
getTemperature()
method doesn't specify whether it returns the temperature in Celsius or Fahrenheit. This could lead to confusion for code that calls this API.- float getTemperature(); + /** + * Gets the current temperature in the configured unit (C or F) + * @return Temperature in the unit specified by degC setting + */ + float getTemperature();
91-91
: Remove commented-out code.The
connected()
method is commented out. Unless it's being kept for future implementation, it would be cleaner to remove it.- //void connected();
102-103
: Follow consistent camelCase naming convention for methods.Method names should follow consistent camelCase convention.
- void overtempfailure(); - void overtempreset(); + void overtempFailure(); + void overtempReset();
47-53
: Comment correction for lastTemperaturesRequest.The comments for
lastTemperaturesRequest
seem to be copied from another sensor implementation (likely DS18B20) that requires a delay after requesting temperatures. The LM75 typically doesn't need this delay mechanism - it provides temperature readings directly through I2C.- // last time requestTemperatures was called - // used to determine when we can read the sensors temperature - // we have to wait at least 93.75 ms after requestTemperatures() is called + // Tracks the last time temperature was requested from the sensor unsigned long lastTemperaturesRequest;
32-32
: Remove commented-out code initialization.Line 32 contains a commented-out sensor initialization. Unless this is kept as a reference example, it would be cleaner to remove it.
-//Generic_LM75 LM75temperature(0x4F);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
usermods/LM75_Temperature/LM75.cpp
(1 hunks)usermods/LM75_Temperature/LM75.h
(1 hunks)usermods/LM75_Temperature/library.json
(1 hunks)usermods/LM75_Temperature/readme.md
(1 hunks)wled00/const.h
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
wled00/const.h (1)
usermods/LM75_Temperature/LM75.h (1)
USERMOD_ID_LM75TEMPERATURE
(87-87)
🪛 LanguageTool
usermods/LM75_Temperature/readme.md
[style] ~13-~13: To form a complete sentence, be sure to include a subject.
Context: ... which the WLED output is switched off. Can be between 1 and 100 ...
(MISSING_IT_THERE)
[style] ~14-~14: To form a complete sentence, be sure to include a subject.
Context: ...mum 1 towards the shutdown temperature. Can be beween 0 and 99°C | 40°C | ## Comp...
(MISSING_IT_THERE)
[uncategorized] ~45-~45: A comma may be missing after the conjunctive/linking adverb ‘Hence’.
Context: ...LED only allows uint8 values as inputs. Hence the maxium Temperature at capped at 212...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~49-~49: A comma might be missing here.
Context: ...
To avoid flickering of the WLED output the overtemperature feature has a hyste...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...gger hystersis when setting the limits. Still the mod checks the inputs and adjust th...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~49-~49: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...ts. Still the mod checks the inputs and adjust the lower value not to equal or higher ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~57-~57: Possible missing comma found.
Context: ...s of now untested.
During power up the mod tries to scan for the I2C devic...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~57-~57: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...ge shows an error message if the sensor can not be reached.
(CAN_NOT_PREMIUM)
🔇 Additional comments (7)
wled00/const.h (1)
213-213
: Proper integration of new usermod ID.The new ID for the LM75 temperature usermod is correctly placed in the usermod IDs list with an incremental value (58), following the established pattern.
usermods/LM75_Temperature/library.json (1)
1-7
: Library dependency correctly configured.The library.json file properly specifies the dependency on the LM75 I2C temperature sensor library with version ~1.0.3, which is needed for this usermod to function.
usermods/LM75_Temperature/LM75.cpp (4)
41-54
: Threshold validation logic is sound.The code properly validates temperature thresholds, ensuring they're within valid ranges and maintaining the required hysteresis between high and low thresholds to prevent flickering. This is well-implemented.
307-312
: Good use of PROGMEM for string constants.The code correctly uses PROGMEM to store string constants in program memory rather than RAM, which is a good practice for memory-constrained devices like ESP8266/ESP32.
314-315
: Usermod registration is correctly implemented.The usermod is properly registered with WLED using the REGISTER_USERMOD macro, making it available to the WLED system.
5-19
:✅ Verification successful
Ensure WLED API methods are used correctly.
The code uses
toggleOnOff()
andstateUpdated(CALL_MODE_BUTTON)
to control the WLED output. Verify that these methods are called in the correct context and thatbri
is a valid reference to the brightness state in the WLED codebase.
🏁 Script executed:
#!/bin/bash # Check how toggleOnOff and stateUpdated are implemented in WLED rg -A 3 "void toggleOnOff\(" --type cpp rg -A 3 "void stateUpdated\(" --type cpp # Check where bri is defined rg "bri\s*=[^=]" --type cppLength of output: 8869
Let’s verify that
bri
is the global brightness variable and find whereCALL_MODE_BUTTON
is defined.
🏁 Script executed:
#!/bin/bash # Find declaration of global brightness variable 'bri' rg -nE "extern\s+.*\bbri\b" -t h -t cpp rg -nE "\buint8_t\s+bri\b" -t h -t cpp # Find the definition of CALL_MODE_BUTTON rg -n "CALL_MODE_BUTTON" -t h -t cppLength of output: 6645
Further verify that
bri
is the global brightness variable in WLED:
🏁 Script executed:
#!/bin/bash # Locate the global declaration of 'bri' rg -n -E "^uint8_t\s+bri\b" -t h -t cpp rg -n "extern\s+uint8_t\s+bri" -t h -t cppLength of output: 483
🏁 Script executed:
#!/bin/bash # Search for the global 'bri' declaration and definition in the WLED sources rg -n -E "uint8_t\s+bri\b" -t h -t cpp wled00 rg -n -E "bri\s*=" -t cpp wled00Length of output: 180
WLED API usage is correct
The calls to
toggleOnOff()
andstateUpdated(CALL_MODE_BUTTON)
inovertempfailure()
/overtempreset()
mirror WLED’s built-in button logic (seebutton.cpp
). The globalbri
variable indeed reflects the current brightness state, so checkingbri > 0
andbri == 0
is valid.No changes required.
usermods/LM75_Temperature/LM75.h (1)
16-22
: Consider the interaction between temperature units and thresholds.The class tracks temperature unit (Celsius/Fahrenheit) but threshold values are defined as fixed numbers. Make sure there's handling for temperature unit conversion when applying thresholds if the user has selected Fahrenheit display.
Consider how the temperature unit (degC variable) interacts with the auto-off thresholds. Are thresholds always evaluated in Celsius regardless of display preference? If not, ensure proper conversion is handled in the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
usermods/LM75_Temperature/readme.md (3)
46-46
: Add missing comma and fix spelling
Minor grammar fix for clarity.- Hence, the maximum temperature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit below 32 °F to avoid negative values in the inner workings of the mod. + Hence, the maximum temperature is capped at 212 °F or 100 °C. Also, be careful not to set the lower limit below 32 °F to avoid negative values in the inner workings of the mod.
50-51
: Improve grammar in hysteresis remark
Correct subject-verb agreement and clarify phrasing:- Still the mod checks the inputs and adjust the lower value not to be equal or higher than the upper limit. + The mod still checks the inputs and adjusts the lower threshold so it never equals or exceeds the upper limit.
57-58
: Fix spelling of “address”
Ensure consistency and correct spelling in the sensor-scan description:- During power up, the mod tries to scan for the I2C devices under the configured adress. The info page shows an error message if the sensor cannot be reached. + During power-up, the mod scans for I2C devices at the configured address. The info page shows an error message if the sensor cannot be reached.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
usermods/LM75_Temperature/LM75.cpp
(1 hunks)usermods/LM75_Temperature/LM75.h
(1 hunks)usermods/LM75_Temperature/library.json
(1 hunks)usermods/LM75_Temperature/readme.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- usermods/LM75_Temperature/library.json
- usermods/LM75_Temperature/LM75.cpp
- usermods/LM75_Temperature/LM75.h
🧰 Additional context used
🪛 LanguageTool
usermods/LM75_Temperature/readme.md
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...perature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit ...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (2)
usermods/LM75_Temperature/readme.md (2)
1-5
: LGTM: Introduction is clear and concise
The header and opening lines effectively describe the purpose and context of this usermod.
54-54
: LGTM: MQTT note
Clarifying that MQTT support is currently untested is useful for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/LM75_Temperature/readme.md (1)
37-39
: Correct macro name and typo in description
The macroUSERMOD_LM75TEMPERATURE_AUTO_TEMPERATURE_OFF_ENABLED
should align with naming convention and past feedback—rename toUSERMOD_LM75TEMPERATURE_AUTO_OFF_ENABLED
. Also fix “Paremeter” → “Parameter”:- - `-D USERMOD_LM75TEMPERATURE_AUTO_TEMPERATURE_OFF_ENABLED=true` + - `-D USERMOD_LM75TEMPERATURE_AUTO_OFF_ENABLED=true` Parameter true or false
🧹 Nitpick comments (6)
usermods/LM75_Temperature/readme.md (6)
6-8
: Surround tables with blank lines & remove trailing punctuation in heading
Markdown tables should be preceded and followed by a blank line for readability, and headings shouldn’t end with a colon per lint rules.-## Parameters: +## Parameters(Add an empty line after the table as well.)
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
7-7: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
12-14
: Improve determiners in description
Add “the” before “set value” for grammatical correctness:-| Reactivate Temperature | Switches the WLED output back on once the temperature drops below set value. Has a hysteresis ... | 40°C | +| Reactivate Temperature | Switches the WLED output back on once the temperature drops below the set value. Has a hysteresis ... | 40°C |🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: You might be missing the article “the” here.
Context: ...ack on once the temperature drops below set value. Has a hysteresis of minimum 1 to...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
16-19
: Remove trailing punctuation in compilation heading
Per markdown lint, drop the colon in the heading:-## Compilation: +## Compilation🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
16-16: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
44-46
: Fix typos and punctuation in remarks
- Add a comma after “Also”.
- Correct “carefull” → “careful”.
- Hence, the maximum temperature is capped at 212 °F or 100 °C. Also be carefull not to set ... + Hence, the maximum temperature is capped at 212 °F or 100 °C. Also, be careful not to set ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: A comma might be missing here.
Context: ...perature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
49-50
: Verb agreement in hysteresis description
Adjust the verb to match the subject:- Still the mod checks the inputs and adjust the lower value not to be equal ... + Still the mod checks the inputs and adjusts the lower value not to be equal ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~49-~49: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...ts. Still the mod checks the inputs and adjust the lower value not to be equal or high...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
57-57
: Fix spelling of “address”
Correct the misspelling in “configured adress”:- During power up, the mod tries to scan for the I2C devices under the configured adress. + During power up, the mod tries to scan for the I2C devices under the configured address.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/LM75_Temperature/readme.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
usermods/LM75_Temperature/readme.md
[uncategorized] ~14-~14: You might be missing the article “the” here.
Context: ...ack on once the temperature drops below set value. Has a hysteresis of minimum 1 to...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~45-~45: A comma might be missing here.
Context: ...perature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~49-~49: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...ts. Still the mod checks the inputs and adjust the lower value not to be equal or high...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
🪛 markdownlint-cli2 (0.17.2)
usermods/LM75_Temperature/readme.md
6-6: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
7-7: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
16-16: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (8)
usermods/LM75_Temperature/readme.md (8)
1-2
: Header consistency
The top-level heading is clear and correctly describes the usermod.
3-5
: Introduction clarity
The opening paragraphs succinctly explain the mod and its lineage. No changes needed.
21-23
: Measurement interval flag
TheUSERMOD_LM75TEMPERATURE_MEASUREMENT_INTERVAL
flag is documented correctly.
25-27
: Enable flag documentation
TheUSERMOD_LM75TEMPERATURE_ENABLED
flag entry is clear and accurate.
29-31
: High threshold flag
TheUSERMOD_LM75TEMPERATURE_AUTO_OFF_HIGH_THRESHOLD
flag and its range are correctly documented.
33-35
: Low threshold flag
TheUSERMOD_LM75TEMPERATURE_AUTO_OFF_LOW_THRESHOLD
flag entry is also accurate.
41-43
: I2C address flag
TheUSERMOD_LM75TEMPERATURE_I2C_ADDRESS
flag is correctly spelled and documented.
53-53
: MQTT feature note
The remark about MQTT being untested is helpful. Consider marking it as a TODO to revisit once validated.
No functional changes, just corrected formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
usermods/LM75_Temperature/LM75.cpp (1)
54-56
:⚠️ Potential issueI²C bus is never initialised – call
Wire.begin()
before talking to the sensor
Wire.beginTransmission()
assumes the bus is already up, which is not guaranteed inside a user-mod.
This was pointed out in a previous review but is still missing, so initialisation may silently fail on first boot.bool UsermodLM75Temperature::findSensor() { uint8_t devicepresent; - //Let's try to communicate with the LM75 sensor + Wire.begin(); // initialise I²C bus + //Let's try to communicate with the LM75 sensor Wire.beginTransmission(USERMOD_LM75TEMPERATURE_I2C_ADDRESS);
🧹 Nitpick comments (1)
usermods/LM75_Temperature/LM75.cpp (1)
300-301
: Global object name clashes with membertemperature
– consider renamingHaving both a global instance and a class member called
temperature
is legal but confusing when debugging or working with symbol look-ups. Renaming the global instance (e.g.lm75Usermod
) will improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/LM75_Temperature/LM75.cpp
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
usermods/LM75_Temperature/LM75.cpp (1)
Learnt from: Deadeye5589
PR: wled/WLED#4680
File: usermods/LM75_Temperature/LM75.cpp:0-0
Timestamp: 2025-05-08T18:11:56.316Z
Learning: The LM75 Temperature usermod already implements a hysteresis mechanism to prevent rapid on/off cycling by enforcing a separation between high threshold (shutdown) and low threshold (reactivation) temperatures.
🪛 Cppcheck (2.10-2)
usermods/LM75_Temperature/LM75.cpp
[error] 65-65: Unmatched '}'. Configuration
(syntaxError)
There will be 3 temperature usermods if this is merged. Adding to confusion and complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some comments. looks ok in general but needs some cleaning and testing.
} | ||
if (sensorFound && !initDone) DEBUG_PRINTLN(F("Init not completed")); | ||
} | ||
lastMeasurement = millis() - readingInterval + 10000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just set to zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please elaborate. What shall be set to zero? The debug message or the reading interval?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the lastMeasurement time at startup.
<br> | ||
|
||
|
||
- `-D USERMOD_LM75TEMPERATURE_MEASUREMENT_INTERVAL=60000` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
60s is a bit long for temperature monitoring as a default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value was already set to 60 seconds in the regular temperature mod, why I did not change it.
void UsermodLM75Temperature::overtempReset() { | ||
overtempTriggered = false; | ||
if(bri == 0) { | ||
toggleOnOff(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems wrong. if strip is turned off by user, this can turn it back on. not sure there is a better way to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came to the same conclusion, but have no solution for it, which would not impact the main source of WLED.
} | ||
|
||
void UsermodLM75Temperature::appendConfigData() { | ||
if(degC) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this section appears very convoluted. also which parameters are changeable by the user? it does not make sense to have compile time defined variables in the config that cannot be changed by the user in the UM config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok makes sense. you could make degC a dropdown as well.
illegal macro used on a run-time string
Elaborated the fact, that the mod might switch a light on due to an overtemperature event. Also removed remark with untested MQTT, since feature was now tested successfull.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/LM75_Temperature/readme.md (1)
37-38
:⚠️ Potential issueFix build flag macro name and typo
The build flag name and its description are incorrect, which will break compilation or confuse users.Apply this diff to correct the macro and spelling:
- - `-D USERMOD_LM75TEMPERATURE_AUTO_TEMPERATURE_OFF_ENABLED=true` - Paremeter true or false + - `-D USERMOD_LM75TEMPERATURE_AUTO_OFF_ENABLED=true` + Parameter true or false
🧹 Nitpick comments (3)
usermods/LM75_Temperature/readme.md (3)
6-8
: Align Markdown headings, table formatting, and add missing article
Headings should not end with colons, the parameters table needs blank lines around it, and there’s a missing “the” before “set value.”- ## Parameters: + ## Parameters + - | Reactivate Temperature | Switches the WLED output back on once the temperature drops below set value. Has a hysteresis of minimum 1 towards the shutdown temperature. Value can be between 0 and 99°C | 40°C | + | Reactivate Temperature | Switches the WLED output back on once the temperature drops below the set value. Has a hysteresis of minimum 1 towards the shutdown temperature. Value can be between 0 and 99°C | 40°C | - ## Compilation: + ## CompilationAlso applies to: 14-14, 16-16
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
7-7: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
45-57
: Improve clarity and fix typos in Remarks
Address grammar, spelling, and phrasing issues in the Remarks section for better readability.- The °F setting is not optimal to be used in combination with the overtemperature protection feature. WLED only allows uint8 values as inputs. Hence, the maximum temperature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit below 32 °F to avoid negative values in the inner workings of the mod. + The °F setting is not optimal when used with the overtemperature protection feature. WLED only allows uint8 inputs; hence, the maximum temperature is capped at 212 °F (100 °C). Also, be careful not to set the lower limit below 32 °F to avoid negative values in the mod’s internal logic. - For example this can lead to the following behavior. User switches light off. Still an overtemperature event occurs. Since the light is already switched off, the mod does not change the current status. Now the temperature drops. And the mod will reenable the light, switching it on. + For example, this can lead to the following behavior: if the user switches the light off and an overtemperature event occurs, nothing changes. When the temperature drops, the mod will re-enable the light, inadvertently turning it back on. - During power up, the mod tries to scan for the I2C devices under the configured adress. The info page shows an error message if the sensor cannot be reached. + During power-up, the mod scans for I2C devices at the configured address. The info page displays an error message if the sensor cannot be reached.🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...perature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit ...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~53-~53: The word “cross-check” is spelled with a hyphen.
Context: ...an the upper limit.
There is no cross check if the user enables or disables the mas...(CROSS_COMPOUNDS)
[typographical] ~53-~53: After the expression ‘for example’ a comma is usually used.
Context: ...ut during an overtemperature event. For example this can lead to the following behavior...(COMMA_FOR_EXAMPLE)
[uncategorized] ~53-~53: You might be missing the article “the” here.
Context: ...o the following behavior. User switches light off. Still an overtemperature event occ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~53-~53: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...the current status. Now the temperature drops. And the mod will reenable the light, s...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
1-59
: Consider consolidating temperature usermods
Introducing a third separate temperature usermod increases maintenance overhead and may confuse users. Consider refactoring into a single, unified temperature usermod with pluggable sensor backends (OneWire, LM75, etc.) to streamline documentation and functionality.🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: You might be missing the article “the” here.
Context: ...ack on once the temperature drops below set value. Has a hysteresis of minimum 1 to...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...perature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit ...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~53-~53: The word “cross-check” is spelled with a hyphen.
Context: ...an the upper limit.
There is no cross check if the user enables or disables the mas...(CROSS_COMPOUNDS)
[typographical] ~53-~53: After the expression ‘for example’ a comma is usually used.
Context: ...ut during an overtemperature event. For example this can lead to the following behavior...(COMMA_FOR_EXAMPLE)
[uncategorized] ~53-~53: You might be missing the article “the” here.
Context: ...o the following behavior. User switches light off. Still an overtemperature event occ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~53-~53: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...the current status. Now the temperature drops. And the mod will reenable the light, s...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
🪛 markdownlint-cli2 (0.17.2)
6-6: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
7-7: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
16-16: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/LM75_Temperature/readme.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
usermods/LM75_Temperature/readme.md
[uncategorized] ~14-~14: You might be missing the article “the” here.
Context: ...ack on once the temperature drops below set value. Has a hysteresis of minimum 1 to...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...perature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit ...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~53-~53: The word “cross-check” is spelled with a hyphen.
Context: ...an the upper limit.
There is no cross check if the user enables or disables the mas...
(CROSS_COMPOUNDS)
[typographical] ~53-~53: After the expression ‘for example’ a comma is usually used.
Context: ...ut during an overtemperature event. For example this can lead to the following behavior...
(COMMA_FOR_EXAMPLE)
[uncategorized] ~53-~53: You might be missing the article “the” here.
Context: ...o the following behavior. User switches light off. Still an overtemperature event occ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~53-~53: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...the current status. Now the temperature drops. And the mod will reenable the light, s...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
🪛 markdownlint-cli2 (0.17.2)
usermods/LM75_Temperature/readme.md
6-6: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
7-7: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
16-16: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
} | ||
if (sensorFound && !initDone) DEBUG_PRINTLN(F("Init not completed")); | ||
} | ||
lastMeasurement = millis() - readingInterval + 10000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the lastMeasurement time at startup.
} | ||
|
||
void UsermodLM75Temperature::appendConfigData() { | ||
if(degC) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok makes sense. you could make degC a dropdown as well.
Parameter I2C Address of the LM75 IC in Hex 0x** | ||
|
||
## Remarks | ||
The °F setting is not optimal to be used in combination with the overtemperature protection feature. WLED only allows uint8 values as inputs. Hence, the maximum temperature is capped at 212 °F or 100 °C. Also be carefull not to set the lower limit below 32 °F to avoid negative values in the inner workings of the mod. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wled allows also for larger values, I know because in the deep-sleep usermod I can enter much larger numbers. not sure of the exact usage though, I just tried until it worked.
A deviation of the well known temperature usermod.
Rewrote code of the v16 version of the temperature usermod.
Removed single wire sensor and added I2C instead.
Added an automatic overtemperature protection.
Added configuration properties for the overtemperature protection.
Added description for parameters in markdown file.
Added usermod ID 58.
MQTT is untested but should not have changed in comparison to regular temperature usermod.
Summary by CodeRabbit
New Features
Documentation