Tuya - add GitHub issue assistant for unsupported and partially supported devices - PR4#2467
Open
Terdious wants to merge 80 commits intoGladysAssistant:masterfrom
Open
Tuya - add GitHub issue assistant for unsupported and partially supported devices - PR4#2467Terdious wants to merge 80 commits intoGladysAssistant:masterfrom
Terdious wants to merge 80 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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)
328-333:⚠️ Potential issue | 🟡 MinorPreserve backend error details on disconnect failures.
Line 332 currently keeps only
e.message, which can hide API error details already handled elsewhere in this component.💡 Suggested fix
} catch (e) { + const responseMessage = + (e && e.response && e.response.data && e.response.data.message) || (e && e.message) || 'unknown'; this.setState({ tuyaDisconnecting: false, tuyaConnectionStatus: RequestStatus.Error, - tuyaConnectionError: (e && e.message) || 'unknown' + tuyaConnectionError: responseMessage }); }🤖 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 328 - 333, In the catch block inside SetupTab (the disconnect handler that sets tuyaDisconnecting, tuyaConnectionStatus and tuyaConnectionError), preserve the full backend error details instead of only e.message: set tuyaConnectionError to the entire error object or a merged string/object that includes e.message plus backend details (e.response?.data, e.body, or other payload fields) so downstream code/logs can access the original API error; update the assignment that currently uses (e && e.message) || 'unknown' to include these backend fields while keeping a safe fallback.
🧹 Nitpick comments (6)
server/test/services/tuya/lib/utils/tuya.deviceParams.test.js (2)
30-35: Consolidate duplicatedgetParamValueassertions into one table-driven test.These two tests overlap heavily and can be merged to reduce maintenance noise while preserving coverage.
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 overlapping tests for getParamValue should be merged into a single table-driven test: replace the duplicated it blocks with one it that defines an array of cases (inputs and expected outputs) covering existing scenarios (existing param, non-existent param, null params) and iterate over the cases (e.g., cases.forEach) asserting getParamValue(input, key) equals expected; update both occurrences that test getParamValue so they use this single parameterized test and remove the duplicate assertions to keep coverage while reducing redundancy.
30-35: Test title and implementation are slightly misaligned.The test says “using device param constants” but uses
'A'/'B'literals. Consider either renaming the test or usingDEVICE_PARAM_NAMEvalues for consistency.🤖 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 test title for the case in tuya.deviceParams.test.js claims it uses device param constants but currently uses string literals; update the test to be consistent by either renaming the test to remove the “using device param constants” wording or replace the literals 'A' and 'B' with the canonical constant(s) (e.g., DEVICE_PARAM_NAME or other exported device param names) and use those constants when calling getParamValue and in expectations so the test title, inputs and assertions align with the intended usage of DEVICE_PARAM_NAME and getParamValue.server/test/services/tuya/lib/tuya.setValue.test.js (1)
368-407: Consider adding logging assertion to match test description.The test is named "should log disconnect failures..." but doesn't verify that logging occurs. The current assertions verify the function completes successfully without cloud fallback, but adding a logger mock would make the test more complete.
💡 Optional: Add logger mock to verify logging
+ const logger = { warn: sinon.stub(), error: sinon.stub(), debug: sinon.stub() }; const ctx = { connector: { request: sinon.stub() }, gladys: {}, + logger, }; await setValue.call(ctx, device, deviceFeature, 1); expect(connect.calledOnce).to.equal(true); expect(set.calledOnce).to.equal(true); expect(disconnect.calledOnce).to.equal(true); expect(ctx.connector.request.called).to.equal(false); + // Verify disconnect error was logged + expect(logger.warn.called || logger.error.called).to.equal(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.setValue.test.js` around lines 368 - 407, Test name says it should "log disconnect failures" but there's no logger assertion; update the test for setValue to inject a mock logger into the call context (e.g., add ctx.logger = { error: sinon.stub() }) before invoking setValue, then after awaiting setValue assert that ctx.logger.error was called (calledOnce) and that the logged message or first argument includes the disconnect error or the word "disconnect" to ensure the failure was logged; reference the setValue test where connect, set and disconnect stubs are used and the ctx object is created.server/services/tuya/lib/tuya.loadDeviceDetails.js (1)
34-50: Consolidate repeated rejection logging into one helper.The four rejection blocks duplicate the same reason extraction pattern; a small helper will reduce maintenance risk and keep logs consistent.
Refactor sketch
- 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}`); - } + const logRejected = (result, label) => { + if (result.status !== 'rejected') return; + const reason = result.reason && result.reason.message ? result.reason.message : result.reason; + logger.warn(`[Tuya] Failed to load ${label} for ${deviceId}: ${reason}`); + }; + logRejected(specResult, 'specifications'); + logRejected(detailsResult, 'details'); + logRejected(propsResult, 'properties'); + logRejected(modelResult, 'thing model');🤖 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, Create a small helper (e.g., extractRejectionReason or logRejection) to centralize the repeated rejection handling and logging: it should accept the Promise result object (specResult/detailsResult/propsResult/modelResult), the deviceId and a context string (e.g., "specifications", "details", "properties", "thing model"), compute reason using the existing pattern (result.reason && result.reason.message ? result.reason.message : result.reason) and call logger.warn with the same formatted message; then replace the four inline blocks with calls to that helper for specResult, detailsResult, propsResult and modelResult so logging is consistent and DRY.server/services/tuya/lib/mappings/index.js (1)
54-72: Consider adding a clarifying comment for the keyword matching logic.The logic is correct but the combined conditions at lines 66-71 are subtle. The intent is: keyword-based matching only applies when (1) required codes are present, (2) modelName exists, and (3) keywords are defined. A brief inline comment would help future maintainers understand this fallback behavior.
🤖 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 54 - 72, Add a brief inline comment inside the matchDeviceType function explaining the keyword-based fallback: that keyword matching is only evaluated when REQUIRED_CODES are satisfied (or none required), modelName is present, and KEYWORDS are defined—i.e., the combined conditional (hasRequiredCode && modelName && keywords.length > 0) gates the final keywords.some(modelName.includes(...)) check; place the comment immediately above the hasRequiredCode / modelName / keywords length conditional to clarify this intent for future maintainers.server/services/tuya/lib/tuya.localScan.js (1)
2-3: Deep imports into library internals may be fragile.The imports from
@demirdeniz/tuyapi-newgen/lib/configand/lib/message-parseraccess internal library paths that are not exported from the package's main entry point. These imports could break if the library restructures. Since this functionality is necessary for UDP broadcast message parsing in local device discovery, consider pinning the@demirdeniz/tuyapi-newgenversion in package.json and add a comment documenting this dependency on internal APIs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/tuya.localScan.js` around lines 2 - 3, The two deep imports (const UDP_KEY and MessageParser) pull internal paths from `@demirdeniz/tuyapi-newgen` and are fragile; update package.json to pin the exact tested version of `@demirdeniz/tuyapi-newgen` (so future installs keep the same internals) and add an inline comment immediately above the imports referencing UDP_KEY and MessageParser explaining that these are internal APIs, why they are required for UDP local discovery, and warning to revisit if the package is upgraded; also consider wrapping the require calls in a try/catch and logging a clear error so failures to load these internal modules are visible at runtime.
🤖 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.loadDeviceDetails.js`:
- Around line 34-80: The current function (tuya.loadDeviceDetails.js) returns a
seemingly successful device object even when all fetches fail; update the logic
that inspects specResult, detailsResult, propsResult, and modelResult so that if
all of these promise results are rejected (or at minimum the critical
detailsResult is rejected) the function does not return a populated device but
instead surface a hard failure (throw an Error or return null/undefined
consistently) with a clear error message including deviceId and the combined
reasons; implement this check right before computing
specifications/details/properties/modelPayload and use the existing reason
extraction pattern to build the error message so upstream callers can detect and
handle failed loads.
---
Duplicate comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 328-333: In the catch block inside SetupTab (the disconnect
handler that sets tuyaDisconnecting, tuyaConnectionStatus and
tuyaConnectionError), preserve the full backend error details instead of only
e.message: set tuyaConnectionError to the entire error object or a merged
string/object that includes e.message plus backend details (e.response?.data,
e.body, or other payload fields) so downstream code/logs can access the original
API error; update the assignment that currently uses (e && e.message) ||
'unknown' to include these backend fields while keeping a safe fallback.
---
Nitpick comments:
In `@server/services/tuya/lib/mappings/index.js`:
- Around line 54-72: Add a brief inline comment inside the matchDeviceType
function explaining the keyword-based fallback: that keyword matching is only
evaluated when REQUIRED_CODES are satisfied (or none required), modelName is
present, and KEYWORDS are defined—i.e., the combined conditional
(hasRequiredCode && modelName && keywords.length > 0) gates the final
keywords.some(modelName.includes(...)) check; place the comment immediately
above the hasRequiredCode / modelName / keywords length conditional to clarify
this intent for future maintainers.
In `@server/services/tuya/lib/tuya.loadDeviceDetails.js`:
- Around line 34-50: Create a small helper (e.g., extractRejectionReason or
logRejection) to centralize the repeated rejection handling and logging: it
should accept the Promise result object
(specResult/detailsResult/propsResult/modelResult), the deviceId and a context
string (e.g., "specifications", "details", "properties", "thing model"), compute
reason using the existing pattern (result.reason && result.reason.message ?
result.reason.message : result.reason) and call logger.warn with the same
formatted message; then replace the four inline blocks with calls to that helper
for specResult, detailsResult, propsResult and modelResult so logging is
consistent and DRY.
In `@server/services/tuya/lib/tuya.localScan.js`:
- Around line 2-3: The two deep imports (const UDP_KEY and MessageParser) pull
internal paths from `@demirdeniz/tuyapi-newgen` and are fragile; update
package.json to pin the exact tested version of `@demirdeniz/tuyapi-newgen` (so
future installs keep the same internals) and add an inline comment immediately
above the imports referencing UDP_KEY and MessageParser explaining that these
are internal APIs, why they are required for UDP local discovery, and warning to
revisit if the package is upgraded; also consider wrapping the require calls in
a try/catch and logging a clear error so failures to load these internal modules
are visible at runtime.
In `@server/test/services/tuya/lib/tuya.setValue.test.js`:
- Around line 368-407: Test name says it should "log disconnect failures" but
there's no logger assertion; update the test for setValue to inject a mock
logger into the call context (e.g., add ctx.logger = { error: sinon.stub() })
before invoking setValue, then after awaiting setValue assert that
ctx.logger.error was called (calledOnce) and that the logged message or first
argument includes the disconnect error or the word "disconnect" to ensure the
failure was logged; reference the setValue test where connect, set and
disconnect stubs are used and the ctx object is created.
In `@server/test/services/tuya/lib/utils/tuya.deviceParams.test.js`:
- Around line 30-35: The two overlapping tests for getParamValue should be
merged into a single table-driven test: replace the duplicated it blocks with
one it that defines an array of cases (inputs and expected outputs) covering
existing scenarios (existing param, non-existent param, null params) and iterate
over the cases (e.g., cases.forEach) asserting getParamValue(input, key) equals
expected; update both occurrences that test getParamValue so they use this
single parameterized test and remove the duplicate assertions to keep coverage
while reducing redundancy.
- Around line 30-35: The test title for the case in tuya.deviceParams.test.js
claims it uses device param constants but currently uses string literals; update
the test to be consistent by either renaming the test to remove the “using
device param constants” wording or replace the literals 'A' and 'B' with the
canonical constant(s) (e.g., DEVICE_PARAM_NAME or other exported device param
names) and use those constants when calling getParamValue and in expectations so
the test title, inputs and assertions align with the intended usage of
DEVICE_PARAM_NAME and getParamValue.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
front/src/config/i18n/de.jsonfront/src/routes/integration/all/tuya/setup-page/SetupTab.jsxserver/services/tuya/lib/mappings/index.jsserver/services/tuya/lib/tuya.disconnect.jsserver/services/tuya/lib/tuya.init.jsserver/services/tuya/lib/tuya.loadDeviceDetails.jsserver/services/tuya/lib/tuya.localPoll.jsserver/services/tuya/lib/tuya.localScan.jsserver/services/tuya/lib/tuya.poll.jsserver/services/tuya/lib/tuya.setValue.jsserver/test/services/tuya/lib/tuya.localPoll.test.jsserver/test/services/tuya/lib/tuya.setValue.test.jsserver/test/services/tuya/lib/utils/tuya.deviceParams.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- server/test/services/tuya/lib/tuya.localPoll.test.js
- server/services/tuya/lib/tuya.setValue.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/test/services/tuya/lib/tuya.poll.test.js (2)
112-125: Tighten this test to pin cloud-call behavior as well.The test title targets “missing code from payload”, but it currently only checks
emit. Add a cloud request assertion so regressions that accidentally skip the cloud status fetch don’t pass silently.Proposed assertion addition
it('should skip cloud feature when code is missing from payload', async () => { await tuyaHandler.poll({ external_id: 'tuya:device', features: [ { external_id: 'tuya:device:missing_code', category: 'light', type: 'binary', }, ], }); + assert.callCount(tuyaHandler.connector.request, 1); + assert.calledWith(tuyaHandler.connector.request, { + method: 'GET', + path: `${API.VERSION_1_0}/devices/device/status`, + }); assert.callCount(gladys.event.emit, 0); });🤖 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 112 - 125, The test verifies no local events are emitted when a feature payload is missing its code but doesn't assert cloud-call behavior; update the test that calls tuyaHandler.poll to also assert the Tuya cloud fetch was not invoked by checking the appropriate stub (e.g., assert.callCount(tuyaApi.getDevice, 0) or assert.notCalled(tuyaApi.getDevice)) so regressions that skip or perform the cloud request will fail; locate the test around tuyaHandler.poll and gladys.event.emit and add the cloud-request assertion referencing the tuyaApi.getDevice (or the actual cloud-fetch helper used in the test harness) stub used in setup.
139-217: Consider extracting repeated Tuya device fixtures into helpers.The repeated
params/featuressetup makes these tests harder to evolve and review. A small builder (e.g.,buildLocalDevice({ features, localOverride })) would reduce duplication and drift risk.Also applies to: 219-283, 429-525
🤖 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 139 - 217, Tests for poll/local polling duplicate the same device fixture across multiple it blocks; extract a helper (e.g., buildLocalDevice) that returns the device object with params and features given options (features array, localOverride boolean, protocolVersion, etc.), replace inline device objects in the tests that call poll (references: poll, localPoll, request, gladys.event.emit) with calls to buildLocalDevice to reduce duplication, and add the helper either at top of the test file or in a shared test helper module and reuse it in the other affected test ranges (also update tests around the other duplicates mentioned).
🤖 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/device/tuya.deviceMapping.js`:
- Around line 19-26: scaleValue currently returns NaN when
Number(valueFromDevice) is not a number; change the behavior to return a
sentinel (null) and emit a warning so upstream code can detect an unparseable
value. In the scaleValue function replace the Number.isNaN(parsedValue) branch
to log a warning (use the module's logger if available, otherwise console.warn)
including valueFromDevice and deviceFeature, and return null; keep the rest of
the function (use of getScale and scaling) unchanged and ensure callers of
scaleValue can handle null as "unparseable".
---
Nitpick comments:
In `@server/test/services/tuya/lib/tuya.poll.test.js`:
- Around line 112-125: The test verifies no local events are emitted when a
feature payload is missing its code but doesn't assert cloud-call behavior;
update the test that calls tuyaHandler.poll to also assert the Tuya cloud fetch
was not invoked by checking the appropriate stub (e.g.,
assert.callCount(tuyaApi.getDevice, 0) or assert.notCalled(tuyaApi.getDevice))
so regressions that skip or perform the cloud request will fail; locate the test
around tuyaHandler.poll and gladys.event.emit and add the cloud-request
assertion referencing the tuyaApi.getDevice (or the actual cloud-fetch helper
used in the test harness) stub used in setup.
- Around line 139-217: Tests for poll/local polling duplicate the same device
fixture across multiple it blocks; extract a helper (e.g., buildLocalDevice)
that returns the device object with params and features given options (features
array, localOverride boolean, protocolVersion, etc.), replace inline device
objects in the tests that call poll (references: poll, localPoll, request,
gladys.event.emit) with calls to buildLocalDevice to reduce duplication, and add
the helper either at top of the test file or in a shared test helper module and
reuse it in the other affected test ranges (also update tests around the other
duplicates mentioned).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
server/services/tuya/lib/device/tuya.convertDevice.jsserver/services/tuya/lib/device/tuya.convertFeature.jsserver/services/tuya/lib/device/tuya.deviceMapping.jsserver/services/tuya/lib/mappings/index.jsserver/test/services/tuya/lib/device/feature/tuya.convertFeature.test.jsserver/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.jsserver/test/services/tuya/lib/device/tuya.convertDevice.test.jsserver/test/services/tuya/lib/device/tuya.localMapping.test.jsserver/test/services/tuya/lib/mappings/index.test.jsserver/test/services/tuya/lib/tuya.poll.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx (2)
607-621:⚠️ Potential issue | 🟠 MajorPreserve server-returned params when updating
PROTOCOL_VERSION.
newParamsis built from pre-requestparams, then assigned tobaseDevice, which can drop backend updates returned inlatestDevice.params.Suggested fix
- const newParams = [...params]; + const baseDevice = latestDevice || currentDevice; + const baseParams = Array.isArray(baseDevice.params) ? baseDevice.params : params; + const paramsByName = new Map( + baseParams.filter(param => param && param.name).map(param => [param.name, param]) + ); if (usedProtocol) { - const protocolIndex = newParams.findIndex(param => param.name === 'PROTOCOL_VERSION'); - if (protocolIndex >= 0) { - newParams[protocolIndex] = { ...newParams[protocolIndex], value: usedProtocol }; - } else { - newParams.push({ name: 'PROTOCOL_VERSION', value: usedProtocol }); - } + const previous = paramsByName.get('PROTOCOL_VERSION') || { name: 'PROTOCOL_VERSION' }; + paramsByName.set('PROTOCOL_VERSION', { ...previous, value: usedProtocol }); } - const baseDevice = latestDevice || currentDevice; + const newParams = Array.from(paramsByName.values()); this.setState({ device: { ...baseDevice, params: newParams🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 607 - 621, The code builds newParams from the pre-request params variable which can drop server-returned updates on latestDevice.params; change the source to start from baseDevice.params (i.e., use latestDevice.params if present, otherwise params) before applying the PROTOCOL_VERSION update using usedProtocol so you preserve backend-updated entries; update the logic around newParams, protocolIndex, and the setState call that assigns device.params to ensure you merge/modify baseDevice.params rather than the stale params array.
454-458:⚠️ Potential issue | 🟠 MajorAdd a null guard for
nextProps.devicebefore dereferencing.Line 456 can throw if
nextProps.deviceis temporarily undefined/null during refresh transitions.Suggested fix
componentWillReceiveProps(nextProps) { const currentDevice = this.state.device; const nextDevice = nextProps.device; + if (!nextDevice) { + return; + } const isNewDevice = !currentDevice || currentDevice.external_id !== nextDevice.external_id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 454 - 458, The code dereferences nextProps.device when computing isNewDevice and shouldRefreshBaseline, which can throw if nextProps.device is null/undefined; update the logic in TuyaDeviceBox (the currentDevice, nextDevice, isNewDevice, shouldRefreshBaseline calculations) to guard nextProps.device before accessing external_id and updated_at—either assign nextDevice = nextProps.device || null and check for null before comparing, or early-return/skip the refresh when nextProps.device is falsy; ensure comparisons use null-safe checks (e.g., only read nextDevice.external_id and nextDevice.updated_at after confirming nextDevice is non-null) so isNewDevice and shouldRefreshBaseline never dereference undefined.front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)
331-336:⚠️ Potential issue | 🟡 MinorKeep API-provided error details in disconnect failures.
Line 335 currently hides backend
response.data.messageand may show less actionable errors.Suggested fix
} catch (e) { + const responseMessage = + (e && e.response && e.response.data && e.response.data.message) || (e && e.message) || 'unknown'; this.setState({ tuyaDisconnecting: false, tuyaConnectionStatus: RequestStatus.Error, - tuyaConnectionError: (e && e.message) || 'unknown' + tuyaConnectionError: responseMessage }); }🤖 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 331 - 336, The catch block in SetupTab.jsx that sets tuyaConnectionError currently uses (e && e.message) || 'unknown' and hides API-provided messages; update the error extraction in that catch (the handler that sets tuyaDisconnecting, tuyaConnectionStatus, and tuyaConnectionError) to prefer the backend message (e.response?.data?.message or e.response && e.response.data && e.response.data.message), then fall back to e.message and then 'unknown' so API-provided error details are preserved.
🧹 Nitpick comments (1)
server/test/services/tuya/lib/tuya.localScan.test.js (1)
9-31: Consider extracting a shared socket-stub factory.The socket mock setup is duplicated across multiple tests (Line 9-31, Line 83-98, Line 151-164, Line 200-227, Line 256-274). A helper would reduce noise and make per-test behavior overrides clearer.
Also applies to: 83-98, 151-164, 200-227, 256-274
🤖 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 9 - 31, Extract the duplicated socket mock into a shared factory function (e.g., makeDgramStub or createSocketStub) and replace each inline dgramStub definition with a call to that factory; the factory should return the same shape used by the tests (createSocket -> returns socket with on, bind, close, handlers and pushes socket into the existing sockets array) and accept optional override callbacks or properties so individual tests can customize handlers or bind behavior without duplicating code; update usages that reference dgramStub.createSocket, sockets, and the socket.handlers to use the new factory-returned object so existing tests keep their behavior.
🤖 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 495-502: Replace the non-interactive <span> used for toggling
visibility with a real keyboard-accessible <button type="button">: move the
onClick handler (this.toggleClientSecret) onto a <button type="button"> and
convert any HTML attributes to React props (e.g., class → className) for both
the button and the nested <i> element; keep the existing conditional class logic
using cx and the state reference (state.showClientSecret) so the eye/eye-off
icon behavior remains unchanged.
In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx`:
- Around line 1144-1155: The input currently renders the raw localKey in clear
text (input id `local_key_${deviceIndex}`, prop `value={localKey}`) which risks
accidental exposure; change to a masked-by-default field with a reveal toggle:
add a local state flag (e.g., `showLocalKey`) and render the input as
type={showLocalKey ? "text" : "password"} (or use an obfuscated display) plus a
small button/icon to toggle `showLocalKey`; ensure the input uses proper JSX
props (`className="form-control"` and `disabled` boolean) and avoid placing the
full key in any data- attributes or visible DOM nodes when `showLocalKey` is
false.
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 148-150: The reader call for each feature can throw and currently
aborts the whole poll; wrap the per-feature reader(...) invocation in a
try/catch so a thrown error for one deviceFeature is caught, logged (include
deviceFeature and error), and that feature is skipped (do not call
emitFeatureState) while allowing the loop to continue; apply the same
per-feature guard around the other reader use site that mirrors lines 247-255 so
each feature is isolated from failures.
- Around line 189-193: The debug log currently prints raw IPs; update the
logger.debug call in tuya.poll.js (the statement that references topic,
requestedMode, hasLocalConfig, protocolVersion, and ipAddress) to avoid emitting
full IPs by masking or omitting ipAddress—e.g., replace the full ipAddress with
a redacted value or a masked form (keep only the first three octets for IPv4 or
hide everything as "<redacted>" when ipAddress is falsy) before interpolating
into the log string so logs no longer contain raw local IPs.
In `@server/test/services/tuya/lib/tuya.localScan.test.js`:
- Around line 253-293: The test currently only awaits localScan(1) resolution;
change it to capture the resolved value (const result = await promise) and add
assertions that validate the response contract: assert Array.isArray(result),
that it contains the parsed message object(s) returned by
MessageParserStub.parse (e.g. an object with payload === null), and that each
item includes the listening socket info from the dgramStub (address() returning
'0.0.0.0' and port 6666) so regressions to the returned shape are caught;
reference localScan, MessageParserStub, dgramStub and the sockets array to
locate where to add these assertions.
---
Duplicate comments:
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 331-336: The catch block in SetupTab.jsx that sets
tuyaConnectionError currently uses (e && e.message) || 'unknown' and hides
API-provided messages; update the error extraction in that catch (the handler
that sets tuyaDisconnecting, tuyaConnectionStatus, and tuyaConnectionError) to
prefer the backend message (e.response?.data?.message or e.response &&
e.response.data && e.response.data.message), then fall back to e.message and
then 'unknown' so API-provided error details are preserved.
In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx`:
- Around line 607-621: The code builds newParams from the pre-request params
variable which can drop server-returned updates on latestDevice.params; change
the source to start from baseDevice.params (i.e., use latestDevice.params if
present, otherwise params) before applying the PROTOCOL_VERSION update using
usedProtocol so you preserve backend-updated entries; update the logic around
newParams, protocolIndex, and the setState call that assigns device.params to
ensure you merge/modify baseDevice.params rather than the stale params array.
- Around line 454-458: The code dereferences nextProps.device when computing
isNewDevice and shouldRefreshBaseline, which can throw if nextProps.device is
null/undefined; update the logic in TuyaDeviceBox (the currentDevice,
nextDevice, isNewDevice, shouldRefreshBaseline calculations) to guard
nextProps.device before accessing external_id and updated_at—either assign
nextDevice = nextProps.device || null and check for null before comparing, or
early-return/skip the refresh when nextProps.device is falsy; ensure comparisons
use null-safe checks (e.g., only read nextDevice.external_id and
nextDevice.updated_at after confirming nextDevice is non-null) so isNewDevice
and shouldRefreshBaseline never dereference undefined.
---
Nitpick comments:
In `@server/test/services/tuya/lib/tuya.localScan.test.js`:
- Around line 9-31: Extract the duplicated socket mock into a shared factory
function (e.g., makeDgramStub or createSocketStub) and replace each inline
dgramStub definition with a call to that factory; the factory should return the
same shape used by the tests (createSocket -> returns socket with on, bind,
close, handlers and pushes socket into the existing sockets array) and accept
optional override callbacks or properties so individual tests can customize
handlers or bind behavior without duplicating code; update usages that reference
dgramStub.createSocket, sockets, and the socket.handlers to use the new
factory-returned object so existing tests keep their behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
front/src/routes/integration/all/tuya/TuyaDeviceBox.jsxfront/src/routes/integration/all/tuya/setup-page/SetupTab.jsxserver/services/tuya/lib/mappings/index.jsserver/services/tuya/lib/tuya.poll.jsserver/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.jsserver/test/services/tuya/lib/mappings/index.test.jsserver/test/services/tuya/lib/tuya.localPoll.test.jsserver/test/services/tuya/lib/tuya.localScan.test.jsserver/test/services/tuya/lib/tuya.poll.test.jsserver/test/services/tuya/lib/utils/tuya.deviceParams.test.js
9 tasks
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
Summary
Details
Scope
Translate
Front
Notes
master, GitHub may show additional changes from previous stacked PRs.Summary by CodeRabbit
New Features
Improvements
Localization