Tuya - add mapping core for local/cloud device handling - PR3#2466
Open
Terdious wants to merge 67 commits intoGladysAssistant:masterfrom
Open
Tuya - add mapping core for local/cloud device handling - PR3#2466Terdious wants to merge 67 commits intoGladysAssistant:masterfrom
Terdious wants to merge 67 commits intoGladysAssistant:masterfrom
Conversation
…er models - Added support for air conditioning devices with new mappings and DPS configurations. - Introduced local polling for Tuya devices to improve responsiveness. - Enhanced device conversion logic to include additional parameters such as cloud IP and local override. - Updated feature conversion to utilize advanced DPS mappings for air conditioning. - Implemented new models for air conditioning and power meter, including specific feature mappings. - Improved error handling and logging for local polling and device value setting. - Added unit tests for new feature mappings and conversion logic.
…y device conversion logic
…ttre à jour les tests associés
… champ d'erreur dans le payload de l'événement WebSocket
…s et ajouter des tests pour la gestion des appareils locaux
…on Tuya dans les fichiers de langue
…ion Tuya et mettre à jour les traductions
… des appareils Tuya
…tre l'utilisation d'adresses spécifiques
…eurs de port et mise à jour des traductions
…re des liens vers la documentation et les options de connexion
…réation de rapports GitHub pour les appareils Tuya
…l disconnect features - Added new translations for connection status messages in German, English, and French. - Implemented API endpoints to get Tuya connection status and to manually disconnect from the Tuya cloud. - Updated the Tuya service to handle automatic reconnection logic and manual disconnect state. - Enhanced the SetupTab component to reflect connection status and provide a disconnect button. - Added tests for the new functionality, including status retrieval and manual disconnect.
- Implemented device ranking and sorting in DiscoverTab for better user experience. - Added loading indicators and improved UI feedback during device scanning. - Refactored local polling logic to update discovered devices with local information. - Introduced utility functions for managing device parameters, including upserting and normalizing values. - Enhanced local scan response handling to merge existing device parameters. - Updated tests to cover new functionality and ensure reliability of device management.
…ls et des tests associés
… débogage pour la gestion des appareils
…age, ajouter des tests pour la reconnexion automatique et la découverte des appareils
… des paramètres dans le code de configuration Tuya
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (3)
server/services/tuya/lib/tuya.poll.js (2)
102-154:⚠️ Potential issue | 🟠 MajorGuard feature iteration against non-array payloads.
Line 126 calls
deviceFeatures.forEachwithout verifying thatdeviceFeaturesis an array. While line 104 checks forArray.isArrayto setsummary.polled, a non-array payload will still throw at line 126. This was flagged in a previous review.🛠️ Proposed defensive fix
const pollCloudFeatures = async function pollCloudFeatures(deviceFeatures, topic) { const summary = { polled: Array.isArray(deviceFeatures) ? deviceFeatures.length : 0, handled: 0, changed: 0, missing: 0, skipped: 0, }; + if (!Array.isArray(deviceFeatures) || deviceFeatures.length === 0) { + return summary; + } + if (!this.connector || typeof this.connector.request !== 'function') { logger.warn(`[Tuya][poll][cloud] connector unavailable for device=${topic}`); return summary; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/tuya.poll.js` around lines 102 - 154, The pollCloudFeatures function currently calls deviceFeatures.forEach and can throw if deviceFeatures is not an array; update pollCloudFeatures to guard the iteration by either returning early when deviceFeatures is not an array (consistent with how summary.polled is computed) or by iterating over Array.isArray(deviceFeatures) ? deviceFeatures : [] so the loop never runs on non-array payloads; reference the pollCloudFeatures function and the deviceFeatures usage to locate where to add the guard.
225-255:⚠️ Potential issue | 🟠 MajorGuard
device.featuresiteration against non-array payloads.
device.features.forEachat line 225 assumesdevice.featuresis always an array. A malformed device object could throw and break polling for that device. This was flagged in a previous review alongside thepollCloudFeaturesguard.🛠️ Proposed defensive fix
+ const deviceFeatures = Array.isArray(device.features) ? device.features : []; const pendingCloudFeatures = []; - device.features.forEach((deviceFeature) => { + deviceFeatures.forEach((deviceFeature) => { const code = getFeatureCode(deviceFeature);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/tuya.poll.js` around lines 225 - 255, The loop assumes device.features is always an array and can throw for malformed devices; before calling device.features.forEach in the polling routine, add a guard using Array.isArray(device.features) (or coerce to an empty array) so the code iterates only when features is an array; locate the snippet where device.features.forEach is used (alongside helpers like getFeatureCode, getLocalDpsFromCode, getFeatureReader, emitFeatureState, getCurrentFeatureState) and either early-return for bad payloads or replace the forEach with iteration over a safe const features = Array.isArray(device.features) ? device.features : [].server/services/tuya/lib/mappings/index.js (1)
115-131:⚠️ Potential issue | 🟠 MajorInclude
specifications.propertiesin code extraction.This function still only extracts codes from
specifications.functionsandspecifications.status, missing codes that may appear inspecifications.properties. This was flagged in a previous review and remains unaddressed.🔧 Proposed fix
const extractCodesFromSpecifications = (specifications) => { const codes = new Set(); if (!specifications || typeof specifications !== 'object') { return codes; } const functions = Array.isArray(specifications.functions) ? specifications.functions : []; const status = Array.isArray(specifications.status) ? specifications.status : []; + const properties = Array.isArray(specifications.properties) ? specifications.properties : []; - [...functions, ...status].forEach((item) => { + [...functions, ...status, ...properties].forEach((item) => { if (!item || !item.code) { return; } codes.add(String(item.code).toLowerCase()); }); return codes; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/mappings/index.js` around lines 115 - 131, The extractCodesFromSpecifications function currently only reads specifications.functions and specifications.status; update it to also read specifications.properties (e.g., const properties = Array.isArray(specifications.properties) ? specifications.properties : []) and include those items when iterating so codes from properties are captured (convert item.code to string, toLowerCase(), and add to the codes Set, same as for functions/status) while keeping the existing null/undefined checks.
🧹 Nitpick comments (1)
server/services/tuya/lib/mappings/index.js (1)
57-79: Verify that emptyREQUIRED_CODESis intentionally permissive.At line 73,
hasRequiredCodeistruewhenrequiredCodes.size === 0, meaning device types without required codes will match based solely on modelName keywords. This may cause unintended matches if a future device type definition omitsREQUIRED_CODES.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/mappings/index.js` around lines 57 - 79, The matchDeviceType function currently treats an empty REQUIRED_CODES set as permissive (hasRequiredCode true), causing matches based solely on modelName; change this so empty REQUIRED_CODES does not satisfy the requirement—update the logic in matchDeviceType (use requiredCodes and hasRequiredCode) to require requiredCodes.size > 0 and at least one code match (e.g., hasRequiredCode = requiredCodes.size > 0 && Array.from(requiredCodes).some(code => codes.has(code))); alternatively, if permissive empty behavior is desired, make it explicit by introducing a flag (e.g., ALLOW_EMPTY_REQUIRED_CODES) on the typeDefinition and check that flag when computing hasRequiredCode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/services/tuya/lib/mappings/index.js`:
- Around line 115-131: The extractCodesFromSpecifications function currently
only reads specifications.functions and specifications.status; update it to also
read specifications.properties (e.g., const properties =
Array.isArray(specifications.properties) ? specifications.properties : []) and
include those items when iterating so codes from properties are captured
(convert item.code to string, toLowerCase(), and add to the codes Set, same as
for functions/status) while keeping the existing null/undefined checks.
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 102-154: The pollCloudFeatures function currently calls
deviceFeatures.forEach and can throw if deviceFeatures is not an array; update
pollCloudFeatures to guard the iteration by either returning early when
deviceFeatures is not an array (consistent with how summary.polled is computed)
or by iterating over Array.isArray(deviceFeatures) ? deviceFeatures : [] so the
loop never runs on non-array payloads; reference the pollCloudFeatures function
and the deviceFeatures usage to locate where to add the guard.
- Around line 225-255: The loop assumes device.features is always an array and
can throw for malformed devices; before calling device.features.forEach in the
polling routine, add a guard using Array.isArray(device.features) (or coerce to
an empty array) so the code iterates only when features is an array; locate the
snippet where device.features.forEach is used (alongside helpers like
getFeatureCode, getLocalDpsFromCode, getFeatureReader, emitFeatureState,
getCurrentFeatureState) and either early-return for bad payloads or replace the
forEach with iteration over a safe const features =
Array.isArray(device.features) ? device.features : [].
---
Nitpick comments:
In `@server/services/tuya/lib/mappings/index.js`:
- Around line 57-79: The matchDeviceType function currently treats an empty
REQUIRED_CODES set as permissive (hasRequiredCode true), causing matches based
solely on modelName; change this so empty REQUIRED_CODES does not satisfy the
requirement—update the logic in matchDeviceType (use requiredCodes and
hasRequiredCode) to require requiredCodes.size > 0 and at least one code match
(e.g., hasRequiredCode = requiredCodes.size > 0 &&
Array.from(requiredCodes).some(code => codes.has(code))); alternatively, if
permissive empty behavior is desired, make it explicit by introducing a flag
(e.g., ALLOW_EMPTY_REQUIRED_CODES) on the typeDefinition and check that flag
when computing hasRequiredCode.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2466 +/- ##
==========================================
+ Coverage 98.79% 98.85% +0.05%
==========================================
Files 1006 1019 +13
Lines 17478 18387 +909
==========================================
+ Hits 17268 18177 +909
Misses 210 210 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
server/services/tuya/lib/tuya.poll.js (1)
99-106:⚠️ Potential issue | 🟠 MajorGuard feature iteration in both cloud and local paths.
Line 123 and Line 222 assume arrays and call
.forEachdirectly. Ifdevice.featuresis malformed, polling throws and cloud fallback on Line 281 can fail for the same reason.🛠️ Proposed defensive fix
const pollCloudFeatures = async function pollCloudFeatures(deviceFeatures, topic) { const summary = { polled: Array.isArray(deviceFeatures) ? deviceFeatures.length : 0, handled: 0, changed: 0, missing: 0, skipped: 0, }; + + if (!Array.isArray(deviceFeatures) || deviceFeatures.length === 0) { + return summary; + } if (!this.connector || typeof this.connector.request !== 'function') { logger.warn(`[Tuya][poll][cloud] connector unavailable for device=${topic}`); return summary; } @@ async function poll(device) { @@ const params = device.params || []; + const deviceFeatures = Array.isArray(device.features) ? device.features : []; @@ - device.features.forEach((deviceFeature) => { + deviceFeatures.forEach((deviceFeature) => { @@ - cloudSummary = await pollCloudFeatures.call(this, device.features, topic); + cloudSummary = await pollCloudFeatures.call(this, deviceFeatures, topic);Also applies to: 123-123, 222-223, 281-281
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/tuya.poll.js` around lines 99 - 106, Guard array iterations of device features in pollCloudFeatures and its local/cloud fallback paths: check Array.isArray(device.features) (or deviceFeatures passed into pollCloudFeatures) before calling .forEach or otherwise iterate; if not an array, treat as empty (increment summary.missing/skipped as appropriate) or return early. Update the loops referenced around the uses of device.features in the cloud path and local fallback so they never call .forEach on a non-array and ensure summary counters (polled/handled/changed/missing/skipped) are updated consistently when features are malformed.
🧹 Nitpick comments (3)
server/test/services/tuya/lib/tuya.localScan.test.js (2)
311-312: Add explicit assertion foripParamexistence.If
ipParamisundefined, accessing.valuewill throw aTypeErrorwith a confusing message rather than a clear test failure. Adding an explicit existence check improves test diagnostics.💡 Proposed fix
const ipParam = response.devices[0].params.find((param) => param.name === 'IP_ADDRESS'); + expect(ipParam).to.exist; expect(ipParam.value).to.equal('1.1.1.1');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/lib/tuya.localScan.test.js` around lines 311 - 312, Add an explicit assertion that the found parameter exists before accessing its value: after computing ipParam (from response.devices[0].params.find(...)) add a check like expect(ipParam).to.exist or expect(ipParam).to.be.ok to fail with a clear message if the param is missing, then keep the existing expect(ipParam.value).to.equal('1.1.1.1'); so the test reports a clear failure when the IP param isn't present.
253-293: Test exercises code path but lacks assertions on logging behavior.The test description states "should log listening address when available" but there's no assertion verifying that logging actually occurs. Consider adding a spy on the logger to verify the expected log message, or rename the test to clarify it's for coverage purposes.
💡 Proposed enhancement to verify logging
it('should log listening address when available', async () => { + const loggerSpy = sinon.spy(); const sockets = []; const dgramStub = { createSocket: () => { // ... existing socket setup ... }, }; // ... existing parser stub ... const { localScan } = proxyquire('../../../../services/tuya/lib/tuya.localScan', { dgram: dgramStub, '@demirdeniz/tuyapi-newgen/lib/message-parser': { MessageParser: MessageParserStub }, '@demirdeniz/tuyapi-newgen/lib/config': { UDP_KEY: 'key' }, + '../../../utils/logger': { debug: loggerSpy }, }); const clock = sinon.useFakeTimers(); const promise = localScan(1); await clock.tickAsync(1100); await promise; clock.restore(); + + expect(loggerSpy.called).to.be.true; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/lib/tuya.localScan.test.js` around lines 253 - 293, The test exercises localScan but doesn't assert logging; update the test to spy/stub the logger used by the module (e.g., the processLogger or logger instance referenced inside localScan) before calling localScan, then assert that the logger method (info/debug) was called with a message containing the listening address and port '0.0.0.0:6666' (or a substring like '0.0.0.0' and '6666'); alternatively, if you don't want to assert logging, rename the test to reflect it's only exercising the code path for coverage. Ensure you reference the localScan import from '../../../../services/tuya/lib/tuya.localScan' and attach the spy before invoking localScan so the assertion can observe the log call.server/test/services/tuya/lib/tuya.poll.test.js (1)
341-581: Add one regression test for malformedfeaturespayloads.Given the new branch-coverage block, a non-array
featurescase would lock in graceful degradation and prevent runtime regressions.🧪 Suggested test addition
describe('TuyaHandler.poll additional branch coverage', () => { + it('should not throw when features payload is not an array', async () => { + const request = sinon.stub().resolves({ result: [{ code: 'switch_1', value: true }] }); + const emit = sinon.stub(); + const { poll } = proxyquire('../../../../services/tuya/lib/tuya.poll', {}); + + await poll.call( + { + connector: { request }, + gladys: { event: { emit } }, + }, + { + external_id: 'tuya:device', + params: [{ name: 'LOCAL_OVERRIDE', value: false }], + features: null, + }, + ); + + expect(emit.called).to.equal(false); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/lib/tuya.poll.test.js` around lines 341 - 581, Add a regression test to the existing "TuyaHandler.poll additional branch coverage" suite that calls the poll function (poll) with a device whose features property is malformed (e.g., null or an object instead of an array) and asserts that poll does not throw, logs a warning via the injected logger (logger.warn called), does not emit events (gladys.event.emit not called), and does not crash the connector.request; stub connector.request and gladys.event.emit as in the other tests and place the test alongside the other it(...) blocks so malformed features gracefully degrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 118-121: The code currently assumes response.result is an array
and calls forEach, which can throw if response.result is truthy but not an
array; update the parsing in tuya.poll.js to explicitly guard with
Array.isArray(response.result) (or coerce using Array.isArray ? response.result
: []) before iterating so values remains empty/safe when the payload shape is
unexpected; locate the block that builds values (the values = {} declaration and
the forEach over response.result) and replace the direct forEach with an
iteration over a validated array to prevent runtime errors.
---
Duplicate comments:
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 99-106: Guard array iterations of device features in
pollCloudFeatures and its local/cloud fallback paths: check
Array.isArray(device.features) (or deviceFeatures passed into pollCloudFeatures)
before calling .forEach or otherwise iterate; if not an array, treat as empty
(increment summary.missing/skipped as appropriate) or return early. Update the
loops referenced around the uses of device.features in the cloud path and local
fallback so they never call .forEach on a non-array and ensure summary counters
(polled/handled/changed/missing/skipped) are updated consistently when features
are malformed.
---
Nitpick comments:
In `@server/test/services/tuya/lib/tuya.localScan.test.js`:
- Around line 311-312: Add an explicit assertion that the found parameter exists
before accessing its value: after computing ipParam (from
response.devices[0].params.find(...)) add a check like expect(ipParam).to.exist
or expect(ipParam).to.be.ok to fail with a clear message if the param is
missing, then keep the existing expect(ipParam.value).to.equal('1.1.1.1'); so
the test reports a clear failure when the IP param isn't present.
- Around line 253-293: The test exercises localScan but doesn't assert logging;
update the test to spy/stub the logger used by the module (e.g., the
processLogger or logger instance referenced inside localScan) before calling
localScan, then assert that the logger method (info/debug) was called with a
message containing the listening address and port '0.0.0.0:6666' (or a substring
like '0.0.0.0' and '6666'); alternatively, if you don't want to assert logging,
rename the test to reflect it's only exercising the code path for coverage.
Ensure you reference the localScan import from
'../../../../services/tuya/lib/tuya.localScan' and attach the spy before
invoking localScan so the assertion can observe the log call.
In `@server/test/services/tuya/lib/tuya.poll.test.js`:
- Around line 341-581: Add a regression test to the existing "TuyaHandler.poll
additional branch coverage" suite that calls the poll function (poll) with a
device whose features property is malformed (e.g., null or an object instead of
an array) and asserts that poll does not throw, logs a warning via the injected
logger (logger.warn called), does not emit events (gladys.event.emit not
called), and does not crash the connector.request; stub connector.request and
gladys.event.emit as in the other tests and place the test alongside the other
it(...) blocks so malformed features gracefully degrade.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
server/services/tuya/lib/mappings/index.jsserver/services/tuya/lib/tuya.poll.jsserver/test/services/tuya/lib/device/feature/tuya.convertFeature.test.jsserver/test/services/tuya/lib/mappings/index.test.jsserver/test/services/tuya/lib/tuya.localScan.test.jsserver/test/services/tuya/lib/tuya.poll.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- server/test/services/tuya/lib/device/feature/tuya.convertFeature.test.js
- server/services/tuya/lib/mappings/index.js
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)
488-488:⚠️ Potential issue | 🟡 MinorUse
autoCompletecasing in JSX (Line 488).
autocompleteshould beautoCompleteso the prop is correctly handled.Suggested change
- autocomplete="off" + autoComplete="off"#!/bin/bash # Read-only verification: find lowercase JSX autocomplete usage rg -n -C2 '\bautocomplete=' --type js --type jsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` at line 488, In SetupTab.jsx update the JSX attribute name from the lowercase HTML form attribute "autocomplete" to React/JSX camelCase "autoComplete" on the input element inside the SetupTab component (the element currently rendering autocomplete="off"); change the prop there to autoComplete="off" so React recognizes and applies it correctly.
🧹 Nitpick comments (2)
front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (2)
29-34: Move side effects/subscriptions out ofcomponentWillMount.Using network calls and websocket listeners here is fragile with modern React/Preact lifecycle behavior; use
componentDidMountfor these side effects.Suggested change
- componentWillMount() { + componentDidMount() { this.getTuyaConfiguration(); this.getTuyaStatus(); this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.STATUS, this.updateConnectionStatus); this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.ERROR, this.displayConnectionError); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines 29 - 34, The lifecycle side-effects (network calls and websocket subscriptions) are placed in componentWillMount; move them to componentDidMount: call this.getTuyaConfiguration() and this.getTuyaStatus() and register the listeners via this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.STATUS, this.updateConnectionStatus) and addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.ERROR, this.displayConnectionError) inside componentDidMount, and remove or leave componentWillMount empty; ensure any corresponding cleanup (removing those listeners) is done in componentWillUnmount.
492-500: Make the secret-toggle control keyboard-accessible.A clickable
<span>is not keyboard-friendly. Use a<button type="button">witharia-label/aria-pressed.Suggested change
- <span class="input-icon-addon cursor-pointer" onClick={this.toggleClientSecret}> + <button + type="button" + class="input-icon-addon cursor-pointer" + onClick={this.toggleClientSecret} + aria-pressed={state.showClientSecret} + aria-label="Toggle secret visibility" + > <i class={cx('fe', { 'fe-eye': !state.showClientSecret, 'fe-eye-off': state.showClientSecret })} /> - </span> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines 492 - 500, The secret-toggle is a clickable <span> and must be keyboard-accessible: replace the <span class="input-icon-addon cursor-pointer" onClick={this.toggleClientSecret}> with a <button type="button"> that preserves the same classes and onClick handler (toggleClientSecret) and add accessibility attributes such as aria-label (e.g., "Toggle client secret visibility") and aria-pressed={state.showClientSecret}; ensure the inner <i> still uses the cx(...) logic for icon classes and the element remains focusable and styled like the original control.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 133-150: In the SetupTab component's save handler, you're calling
this.setState(...) then immediately reading this.state values (tuyaEndpoint,
tuyaAccessKey, tuyaSecretKey, tuyaAppAccountId, tuyaAppUsername), which can race
with React's async setState; capture the form values into local constants before
calling setState (e.g., const tuyaEndpoint = (this.state.tuyaEndpoint ||
'').trim(); etc.), then call this.setState(...) and use those local constants
for the save logic instead of re-reading this.state.
---
Duplicate comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Line 488: In SetupTab.jsx update the JSX attribute name from the lowercase
HTML form attribute "autocomplete" to React/JSX camelCase "autoComplete" on the
input element inside the SetupTab component (the element currently rendering
autocomplete="off"); change the prop there to autoComplete="off" so React
recognizes and applies it correctly.
---
Nitpick comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 29-34: The lifecycle side-effects (network calls and websocket
subscriptions) are placed in componentWillMount; move them to componentDidMount:
call this.getTuyaConfiguration() and this.getTuyaStatus() and register the
listeners via
this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.STATUS,
this.updateConnectionStatus) and addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.ERROR,
this.displayConnectionError) inside componentDidMount, and remove or leave
componentWillMount empty; ensure any corresponding cleanup (removing those
listeners) is done in componentWillUnmount.
- Around line 492-500: The secret-toggle is a clickable <span> and must be
keyboard-accessible: replace the <span class="input-icon-addon cursor-pointer"
onClick={this.toggleClientSecret}> with a <button type="button"> that preserves
the same classes and onClick handler (toggleClientSecret) and add accessibility
attributes such as aria-label (e.g., "Toggle client secret visibility") and
aria-pressed={state.showClientSecret}; ensure the inner <i> still uses the
cx(...) logic for icon classes and the element remains focusable and styled like
the original control.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
front/src/routes/integration/all/tuya/setup-page/SetupTab.jsxserver/services/tuya/lib/tuya.disconnect.js
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)
488-488:⚠️ Potential issue | 🟡 MinorUse
autoCompleteprop casing in JSX.
autocompleteat Line 488 should beautoCompleteto match JSX prop conventions.#!/bin/bash # Verify JSX prop casing for autocomplete usage in this component. rg -nP '\bautocomplete\s*=' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` at line 488, In the SetupTab.jsx component change the JSX prop name from lowercase "autocomplete" to the correct React/JSX casing "autoComplete" on the input/element where it's currently set (search for the attribute in SetupTab JSX markup); update any other occurrences found by the provided ripgrep command to ensure consistent camelCase prop usage and run the component build/test to verify no props are missed.
🧹 Nitpick comments (4)
server/services/tuya/lib/tuya.loadDeviceDetails.js (1)
34-50: Consider extracting repeated error-logging logic.The reason extraction pattern is duplicated four times. A small helper would reduce repetition.
♻️ Optional refactor
+const logIfRejected = (result, label) => { + if (result.status === 'rejected') { + const reason = result.reason?.message ?? result.reason; + logger.warn(`[Tuya] Failed to load ${label} for ${deviceId}: ${reason}`); + } +}; + +logIfRejected(specResult, 'specifications'); +logIfRejected(detailsResult, 'details'); +logIfRejected(propsResult, 'properties'); +logIfRejected(modelResult, 'thing model'); - if (specResult.status === 'rejected') { - const reason = specResult.reason && specResult.reason.message ? specResult.reason.message : specResult.reason; - logger.warn(`[Tuya] Failed to load specifications for ${deviceId}: ${reason}`); - } - if (detailsResult.status === 'rejected') { - const reason = - detailsResult.reason && detailsResult.reason.message ? detailsResult.reason.message : detailsResult.reason; - logger.warn(`[Tuya] Failed to load details for ${deviceId}: ${reason}`); - } - if (propsResult.status === 'rejected') { - const reason = propsResult.reason && propsResult.reason.message ? propsResult.reason.message : propsResult.reason; - logger.warn(`[Tuya] Failed to load properties for ${deviceId}: ${reason}`); - } - if (modelResult.status === 'rejected') { - const reason = modelResult.reason && modelResult.reason.message ? modelResult.reason.message : modelResult.reason; - logger.warn(`[Tuya] Failed to load thing model for ${deviceId}: ${reason}`); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/tuya.loadDeviceDetails.js` around lines 34 - 50, Extract the repeated reason-extraction and logging into a small helper function (e.g., getRejectionReason(result) and logRejection(result, label, deviceId)) and replace the four duplicate blocks that use specResult, detailsResult, propsResult, and modelResult with calls to that helper; getRejectionReason should return result.reason.message if present or result.reason otherwise, and logRejection should call logger.warn(`[Tuya] Failed to load ${label} for ${deviceId}: ${reason}`) only when result.status === 'rejected', keeping the original labels ("specifications", "details", "properties", "thing model").server/test/services/tuya/lib/utils/tuya.deviceParams.test.js (1)
30-35: Test descriptions are swapped relative to their content.The test at line 30 claims "using device param constants" but actually uses plain string literals (
'A','B'). Conversely, the test at line 51 has a generic description but actually usesDEVICE_PARAM_NAMEconstants. Consider swapping the descriptions to match the actual test content.🔧 Proposed fix
- it('should get param value using device param constants', () => { + it('should get param value', () => { const value = getParamValue([{ name: 'A', value: 42 }], 'A'); expect(value).to.equal(42); expect(getParamValue([{ name: 'A', value: 42 }], 'B')).to.equal(undefined); expect(getParamValue(null, 'A')).to.equal(undefined); });- it('should get param value', () => { + it('should get param value using device param constants', () => { const params = [ { name: DEVICE_PARAM_NAME.IP_ADDRESS, value: '1.1.1.1' }, { name: DEVICE_PARAM_NAME.PROTOCOL_VERSION, value: '3.3' }, ];Also applies to: 51-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/lib/utils/tuya.deviceParams.test.js` around lines 30 - 35, The two unit tests around getParamValue have their descriptions reversed: the test that uses plain string keys ('A', 'B') is labeled to use device param constants and the test that actually uses DEVICE_PARAM_NAME constants has a generic description; update the test descriptions so the one invoking getParamValue([{ name: 'A', value: 42 }], 'A') reads something like "should get param value using plain param names" and the one that passes DEVICE_PARAM_NAME constants reads "should get param value using device param constants" to match the actual test code (refer to the getParamValue calls and DEVICE_PARAM_NAME usage to locate the tests).server/services/tuya/lib/mappings/index.js (1)
137-141: Alignexternal_idcode parsing with the polling path.This parser uses
parts[2], whileserver/services/tuya/lib/tuya.poll.js(getFeatureCode) uses the last segment. Using a fixed index here is more brittle and can drift from polling behavior.♻️ Proposed fix
- const parts = String(feature.external_id).split(':'); - if (parts.length >= 3) { - const code = parts[2]; + const parts = String(feature.external_id).split(':'); + if (parts.length >= 2) { + const code = parts[parts.length - 1]; if (code) { codes.add(String(code).toLowerCase()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/mappings/index.js` around lines 137 - 141, The code in mappings/index.js extracts the feature code using parts[2], which can diverge from the polling implementation getFeatureCode in server/services/tuya/lib/tuya.poll.js that uses the last segment; modify the parser in mappings/index.js (the block that splits feature.external_id and currently reads parts[2]) to instead take the last segment (e.g., parts[parts.length - 1]) and normalize it the same way getFeatureCode does (string coercion and toLowerCase) so both parsers remain consistent.server/test/services/tuya/lib/tuya.localPoll.test.js (1)
16-146: Consider extracting a shared rejection assertion helper.The repeated
try/catch + throw new Error('Expected error')pattern appears across multiple tests. A small helper would reduce duplication and make future error-path additions less noisy.Also applies to: 189-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/lib/tuya.localPoll.test.js` around lines 16 - 146, Tests repeat the same try/catch + throw pattern to assert errors; create a small helper (e.g., assertRejectsWith) that takes a function returning a promise, an expected error class (BadParameters) and expected message or substring, and use it to replace the repeated try/catch blocks in the tests that call localPoll (the tests that currently define TuyAPIStub/TuyAPINewGenStub and then await localPoll with invalid responses). Update each failing-path test (the ones asserting instanceOf BadParameters and message checks) to call assertRejectsWith(() => localPoll({...}), BadParameters, '...') instead of the try/catch + throw to remove duplication while preserving the same assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 56-63: getVariable is swallowing all errors and returning fallback
for backend/network failures; change it to only return fallback for a
404/NotFound (or explicit "variable missing") response and for other errors
either log the error via this.props.logger/processLogger and rethrow or
propagate the exception so the caller can handle it. Locate getVariable and the
similar fetch helpers around the SetupTab component (references: getVariable,
this.props.httpClient.get) and update their catch blocks to inspect the
error/response status: if status indicates missing variable return fallback,
otherwise log the full error and rethrow (or return a rejected Promise). Apply
the same change to the other fetch routines in lines 65–93 so real
network/backend failures are not silently ignored.
In `@server/services/tuya/lib/mappings/index.js`:
- Around line 155-158: The current logic sets codes =
extractCodesFromSpecifications(specifications) and only falls back to
extractCodesFromFeatures(device.features) if that set is empty, which drops
feature-derived codes when specs are partial; change this to merge both sources
instead: call extractCodesFromSpecifications(specifications) and
extractCodesFromFeatures(device.features) and take the union into codes so both
spec and feature codes are considered when selecting mappings (refer to
extractCodesFromSpecifications, extractCodesFromFeatures, the codes variable,
and device.features to locate where to combine the sets).
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 223-224: When localPoll succeeds but localResult.dps is
missing/invalid, set an explicit fallback reason (e.g., fallback =
'invalid-local-dps') before falling back to cloud so logs/tracing show the root
cause; update the branch around the localResult/dps check in tuya.poll.js (the
block referencing localResult, dps and the conditional if (dps && typeof dps ===
'object')) and apply the same change to the analogous block around lines 286-289
so the fallback variable is set to a descriptive value whenever DPS is absent or
not an object.
- Around line 123-125: The loop that assigns values[feature.code] from result
(in the result.forEach callback) can throw when items are null or not objects;
update the result.forEach((feature) => { ... }) callback to validate each item
before using feature.code/value — e.g., skip if feature is falsy or typeof
feature !== 'object' or feature.code is undefined — and only perform
values[feature.code] = feature.value when those checks pass (optionally log or
count skipped items for visibility).
In `@server/test/services/tuya/lib/tuya.localPoll.test.js`:
- Around line 225-258: Rename or update the two failing tests so their
descriptions match what they assert: either change the it(...) titles to
describe that they expect the connect error to be thrown (e.g., for the first
test that currently reads "should log last socket error when different from
thrown error" change to something like "should throw connect error when connect
rejects") or add explicit assertions that the socket error was logged/handled
(mock and assert processLogger.error or the instance.on('error') side effect)
and for the second test (the one around "stop cleanup when already resolved")
similarly align the title with the asserted thrown cleanup error or add
assertions that verify cleanup stopping behavior; update the tests that call
localPoll (the localPoll invocation and TuyAPIStub/setup) and their expect(...)
calls accordingly so test names and asserted behavior match.
- Around line 4-8: Replace the current top-level proxyquire setup and direct
import of updateDiscoveredDeviceAfterLocalPoll by using
proxyquire().noPreserveCache() and removing the direct require of
updateDiscoveredDeviceAfterLocalPoll; instead, defer loading the function via
proxyquire when each test needs it so the module is reloaded with stubs applied.
Specifically, change the proxyquire initialization to call noPreserveCache() on
the proxyquire require and stop importing updateDiscoveredDeviceAfterLocalPoll
at module scope—load it through proxyquire inside the test that needs
updateDiscoveredDeviceAfterLocalPoll so stubs take effect.
---
Duplicate comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Line 488: In the SetupTab.jsx component change the JSX prop name from
lowercase "autocomplete" to the correct React/JSX casing "autoComplete" on the
input/element where it's currently set (search for the attribute in SetupTab JSX
markup); update any other occurrences found by the provided ripgrep command to
ensure consistent camelCase prop usage and run the component build/test to
verify no props are missed.
---
Nitpick comments:
In `@server/services/tuya/lib/mappings/index.js`:
- Around line 137-141: The code in mappings/index.js extracts the feature code
using parts[2], which can diverge from the polling implementation getFeatureCode
in server/services/tuya/lib/tuya.poll.js that uses the last segment; modify the
parser in mappings/index.js (the block that splits feature.external_id and
currently reads parts[2]) to instead take the last segment (e.g.,
parts[parts.length - 1]) and normalize it the same way getFeatureCode does
(string coercion and toLowerCase) so both parsers remain consistent.
In `@server/services/tuya/lib/tuya.loadDeviceDetails.js`:
- Around line 34-50: Extract the repeated reason-extraction and logging into a
small helper function (e.g., getRejectionReason(result) and logRejection(result,
label, deviceId)) and replace the four duplicate blocks that use specResult,
detailsResult, propsResult, and modelResult with calls to that helper;
getRejectionReason should return result.reason.message if present or
result.reason otherwise, and logRejection should call logger.warn(`[Tuya] Failed
to load ${label} for ${deviceId}: ${reason}`) only when result.status ===
'rejected', keeping the original labels ("specifications", "details",
"properties", "thing model").
In `@server/test/services/tuya/lib/tuya.localPoll.test.js`:
- Around line 16-146: Tests repeat the same try/catch + throw pattern to assert
errors; create a small helper (e.g., assertRejectsWith) that takes a function
returning a promise, an expected error class (BadParameters) and expected
message or substring, and use it to replace the repeated try/catch blocks in the
tests that call localPoll (the tests that currently define
TuyAPIStub/TuyAPINewGenStub and then await localPoll with invalid responses).
Update each failing-path test (the ones asserting instanceOf BadParameters and
message checks) to call assertRejectsWith(() => localPoll({...}), BadParameters,
'...') instead of the try/catch + throw to remove duplication while preserving
the same assertions.
In `@server/test/services/tuya/lib/utils/tuya.deviceParams.test.js`:
- Around line 30-35: The two unit tests around getParamValue have their
descriptions reversed: the test that uses plain string keys ('A', 'B') is
labeled to use device param constants and the test that actually uses
DEVICE_PARAM_NAME constants has a generic description; update the test
descriptions so the one invoking getParamValue([{ name: 'A', value: 42 }], 'A')
reads something like "should get param value using plain param names" and the
one that passes DEVICE_PARAM_NAME constants reads "should get param value using
device param constants" to match the actual test code (refer to the
getParamValue calls and DEVICE_PARAM_NAME usage to locate the tests).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
front/src/routes/integration/all/tuya/setup-page/SetupTab.jsxserver/services/tuya/lib/mappings/index.jsserver/services/tuya/lib/tuya.init.jsserver/services/tuya/lib/tuya.loadDeviceDetails.jsserver/services/tuya/lib/tuya.poll.jsserver/test/services/tuya/lib/tuya.localPoll.test.jsserver/test/services/tuya/lib/utils/tuya.deviceParams.test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request check-list
To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:
npm teston both front/server)npm run eslinton both front/server)npm run prettieron both front/server)npm run compare-translationson front)front/src/config/demo.js) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.
Description of change
This PR introduces a mapping core for Tuya devices to make local/cloud handling more consistent and future device integrations easier.
Summary
Details
convertDeviceconvertFeaturetuya.deviceMappingtuya.localMappingScope
Translate
Front
Server
Server Tests
Summary by CodeRabbit
New Features
Improvements