fix online state - #792
Conversation
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix the online state handling when a WLED device is offline by adding connected = false flag when device initialization fails. However, the PR also introduces a problematic change to how object values are serialized in the create_state function.
Changes:
- Sets
connectedflag tofalsewhen device initialization fails (line 1386) - this is a correct fix - Modifies object value serialization logic to extract
.valproperty before stringifying (line 1593) - this introduces critical bugs - Adds changelog entry to README (missing issue reference)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| main.js | Adds connected = false flag when device is offline (good fix) and modifies value serialization in create_state (introduces bugs) |
| README.md | Adds changelog entry for the online state fix |
| if (value != null) { | ||
| await this.setStateChangedAsync(stateName, { | ||
| val: typeof value === 'object' ? JSON.stringify(value) : value, // real objects are not allowed | ||
| val: typeof value === 'object' ? JSON.stringify(value.val) : value, // real objects are not allowed |
There was a problem hiding this comment.
This change introduces two bugs:
-
When value is a state object like
{ val: false, ack: true }(line 1392), the code extractsvalue.valwhich isfalse, then callsJSON.stringify(false)which produces the STRING"false"instead of the booleanfalse. This corrupts the state type. -
When value is a plain object from device data (not a state object), like at lines 956, 960, 1137, the code tries to access
value.valwhich isundefined, then callsJSON.stringify(undefined)which returnsundefined. This breaks state creation for regular device data objects.
The original code JSON.stringify(value) was correct for plain objects. If the goal is to handle state objects passed at line 1392, the fix should be at the call site (pass just false instead of { val: false, ack: true }), not in the generic create_state function. Alternatively, add proper detection: typeof value === 'object' ? (value.val !== undefined && value.ack !== undefined ? value.val : JSON.stringify(value)) : value
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| ### __WORK IN PROGRESS__ | ||
| * (DutchmanNL) **CI/CD**: Fixed deployment failure by adding missing sentry-version-prefix parameter to GitHub Actions workflow | ||
| * (DutchmanNL) **CI/CD**: Updated GitHub Copilot instructions template from v0.4.2 to v0.5.6 - adds ESLint configuration, translation management, lint-first CI/CD workflow guidance | ||
| * (arteck) fix _online state |
There was a problem hiding this comment.
The README changelog entry is missing the issue/PR reference. According to the documentation standards in the guidelines, changelog entries should reference the related issue (e.g., "fixes #XX"). Please add a reference to the issue this PR addresses, such as: * (arteck) fix _online state (fixes #XX)
| * (arteck) fix _online state | |
| * (arteck) fix _online state (fixes #XX) |
if the device is offline (power off) then we have no data from axios response
"false" ist not a booleon
check all possibilities in value and set to boolean (true/false) and not to string