Use extended address in WS API calls #21172
Conversation
This follows the HA Core change and passes the extended address to the OTBR WS API calls where necessary. It also follows the new OTBR info format which potentially includes multiple OTBRs. This allows to support multiple OTBR managed by a single system. Note: There is one corner case when none of the OTBR is found via discovery. In this case we offer to reset the OTBR. Currently we simply offer this for the primary or first one found.
WalkthroughWalkthroughThis update enhances the Changes
Sequence Diagram(s)Updated
|
![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
Outside diff range comments (1)
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts (1)
Line range hint
463-481: Specify a more accurate type thananyforotbr.Using
anyreduces type safety. Consider specifying a more accurate type for better type checking and maintainability.- const otbr = (ev.currentTarget as any).otbr as OTBRInfo; + const otbr = (ev.currentTarget as HTMLElement).otbr as OTBRInfo;Tools
Biome
[error] 472-472: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 487-487: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Outdated
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Outdated
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Outdated
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Show resolved
Hide resolved
…d-config-panel.ts Co-authored-by: Bram Kragten <[email protected]>
…d-config-panel.ts Co-authored-by: Bram Kragten <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range comments (4)
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts (4)
Line range hint
491-504: Improve error handling in_resetBorderRouter.The method uses
anyfor error handling, which reduces type safety. Consider specifying a more accurate type for the error and enhance error handling to manage specific error cases.try { await OTBRCreateNetwork(this.hass, otbr.extended_address); } catch (err: unknown) { showAlertDialog(this, { title: this.hass.localize("ui.panel.config.thread.otbr_config_failed"), text: (err as Error).message, }); // Log error or handle specific error cases here }Tools
Biome
[error] 472-472: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
[error] 487-487: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
Line range hint
612-644: Improve error handling in_changeChannel.The method uses
anyfor error handling, which reduces type safety. Consider specifying a more accurate type for the error and enhance error handling to manage specific error cases.try { const result = await OTBRSetChannel(this.hass, otbr.extended_address, channel); showAlertDialog(this, { title: this.hass.localize("ui.panel.config.thread.change_channel_initiated_title"), text: this.hass.localize("ui.panel.config.thread.change_channel_initiated_text", { delay: Math.floor(result.delay / 60) }), }); } catch (err: unknown) { if (err instanceof CustomError) { // Assume CustomError is a more specific error type you might expect showAlertDialog(this, { title: "Specific Error", text: err.message, }); } else { showAlertDialog(this, { title: "Error", text: (err as Error).message, }); } }
Line range hint
514-533: Improve error handling in_setDataset.The method uses
anyfor error handling, which reduces type safety. Consider specifying a more accurate type for the error and enhance error handling to manage specific error cases.try { await OTBRSetNetwork(this.hass, otbr.extended_address, preferedDatasetId); } catch (err: unknown) { showAlertDialog(this, { title: this.hass.localize("ui.panel.config.thread.otbr_config_failed"), text: (err as Error).message, }); // Log error or handle specific error cases here }Tools
Biome
[error] 533-533: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
Line range hint
654-654: Specify a more accurate type thananyforev.Using
anyreduces type safety. Consider specifying a more accurate type for better type checking and maintainability.- const dataset = (ev.currentTarget as any).networkDataset as ThreadDataSet; + const dataset = (ev.currentTarget as HTMLElement).networkDataset as ThreadDataSet;
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Outdated
Show resolved
Hide resolved
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range comments (2)
src/panels/config/integrations/integration-panels/thread/thread-config-panel.ts (2)
Line range hint
211-254: Complex logic in_renderNetworkneeds refactoring.The function
_renderNetworkcontains complex logic to determine the OTBR for a network. Consider refactoring to reduce complexity and improve maintainability.- const otbr = this._otbrInfo && this._otbrInfo[router.extended_address]; - const showOverflow = ("dataset" in network && router.border_agent_id) || otbr; + const otbr = this._otbrInfo?.[router.extended_address]; + const showOverflow = network.dataset && router.border_agent_id || otbr;Tools
Biome
[error] 212-212: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 210-210: Excessive complexity detected.
Please refactor this function to reduce its complexity score from 31 to the max allowed complexity 15.
(lint/complexity/noExcessiveCognitiveComplexity)
Line range hint
471-497: Specify a more accurate type thananyfor event handling and improve error handling in border router methods.Using
anyreduces type safety. Consider specifying a more accurate type for better type checking and maintainability. Also, add specific error handling for the API call in_resetBorderRouter.- const otbr = (ev.currentTarget as any).otbr as OTBRInfo; + const otbr = (ev.currentTarget as HTMLElement).otbr as OTBRInfo || null; if (!otbr) { return; } try { await OTBRCreateNetwork(this.hass, otbr.extended_address); } catch (err: any) { showAlertDialog(this, { title: this.hass.localize("ui.panel.config.thread.otbr_config_failed"), text: err.message, }); // Log error or handle specific error cases here }Tools
Biome
[error] 478-478: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 493-493: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
| const otbrForNetwork = | ||
| this._otbrInfo && | ||
| network.dataset && | ||
| ((network.dataset.preferred_extended_address && | ||
| this._otbrInfo[network.dataset.preferred_extended_address]) || | ||
| Object.values(this._otbrInfo).find( | ||
| (otbr) => otbr.extended_pan_id === network.dataset!.extended_pan_id | ||
| )); |
There was a problem hiding this comment.
Refactor suggestion: Use optional chaining to simplify access.
The current implementation uses a complex chain of conditions. Simplifying this with optional chaining can enhance readability and safety.
- ((network.dataset.preferred_extended_address && this._otbrInfo[network.dataset.preferred_extended_address]) || Object.values(this._otbrInfo).find((otbr) => otbr.extended_pan_id === network.dataset!.extended_pan_id));
+ network.dataset?.preferred_extended_address ? this._otbrInfo?.[network.dataset.preferred_extended_address] : Object.values(this._otbrInfo ?? {}).find((otbr) => otbr.extended_pan_id === network.dataset?.extended_pan_id);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const otbrForNetwork = | |
| this._otbrInfo && | |
| network.dataset && | |
| ((network.dataset.preferred_extended_address && | |
| this._otbrInfo[network.dataset.preferred_extended_address]) || | |
| Object.values(this._otbrInfo).find( | |
| (otbr) => otbr.extended_pan_id === network.dataset!.extended_pan_id | |
| )); | |
| const otbrForNetwork = | |
| this._otbrInfo && | |
| network.dataset && | |
| (network.dataset?.preferred_extended_address | |
| ? this._otbrInfo?.[network.dataset.preferred_extended_address] | |
| : Object.values(this._otbrInfo ?? {}).find( | |
| (otbr) => otbr.extended_pan_id === network.dataset?.extended_pan_id | |
| )); |
Tools
Biome
[error] 170-170: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
| private _sendCredentials(ev) { | ||
| const otbr = (ev.currentTarget as any).otbr as OTBRInfo; | ||
| if (!otbr) { | ||
| return; | ||
| } | ||
| this.hass.auth.external!.fireMessage({ | ||
| type: "thread/store_in_platform_keychain", | ||
| payload: { | ||
| mac_extended_address: this._otbrInfo.extended_address, | ||
| border_agent_id: this._otbrInfo.border_agent_id ?? "", | ||
| active_operational_dataset: this._otbrInfo.active_dataset_tlvs ?? "", | ||
| mac_extended_address: otbr.extended_address, | ||
| border_agent_id: otbr.border_agent_id ?? "", | ||
| active_operational_dataset: otbr.active_dataset_tlvs ?? "", |
There was a problem hiding this comment.
Use more specific types instead of any and add null checks.
Using any reduces type safety. Consider specifying a more accurate type and adding null checks for better error handling.
- const otbr = (ev.currentTarget as any).otbr as OTBRInfo;
+ const otbr = (ev.currentTarget as HTMLElement).otbr as OTBRInfo || null;
if (!otbr) {
return;
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private _sendCredentials(ev) { | |
| const otbr = (ev.currentTarget as any).otbr as OTBRInfo; | |
| if (!otbr) { | |
| return; | |
| } | |
| this.hass.auth.external!.fireMessage({ | |
| type: "thread/store_in_platform_keychain", | |
| payload: { | |
| mac_extended_address: this._otbrInfo.extended_address, | |
| border_agent_id: this._otbrInfo.border_agent_id ?? "", | |
| active_operational_dataset: this._otbrInfo.active_dataset_tlvs ?? "", | |
| mac_extended_address: otbr.extended_address, | |
| border_agent_id: otbr.border_agent_id ?? "", | |
| active_operational_dataset: otbr.active_dataset_tlvs ?? "", | |
| private _sendCredentials(ev) { | |
| const otbr = (ev.currentTarget as HTMLElement).otbr as OTBRInfo || null; | |
| if (!otbr) { | |
| return; | |
| } | |
| this.hass.auth.external!.fireMessage({ | |
| type: "thread/store_in_platform_keychain", | |
| payload: { | |
| mac_extended_address: otbr.extended_address, | |
| border_agent_id: otbr.border_agent_id ?? "", | |
| active_operational_dataset: otbr.active_dataset_tlvs ?? "", |
Tools
Biome
[error] 337-337: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 341-341: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
| .network=${network} | ||
| .path=${mdiInformationOutline} | ||
| @click=${this._showDatasetInfo} | ||
| ></ha-icon-button | ||
| >${!network.dataset.preferred && !network.routers?.length | ||
| ? html`<ha-icon-button | ||
| label=${this.hass.localize( |
There was a problem hiding this comment.
| label=${this.hass.localize( | |
| .label=${this.hass.localize( |
|
|
||
| return html`<ha-card> | ||
| <div class="card-header"> | ||
| ${network.name}${network.dataset | ||
| ? html`<div> | ||
| <ha-icon-button | ||
| label=${this.hass.localize( |
There was a problem hiding this comment.
| label=${this.hass.localize( | |
| .label=${this.hass.localize( |
frenck
added
the
smash
Breaking change
Proposed change
This follows the HA Core change and passes the extended address to the OTBR WS API calls where necessary. It also follows the new OTBR info format which potentially includes multiple OTBRs.
This allows to support multiple OTBR managed by a single system.
Note: There is one corner case when none of the OTBR is found via discovery. In this case we offer to reset the OTBR. Currently we simply offer this for the primary or first one found.
Related Core PR: home-assistant/core#108282
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
extended_pan_idsupport and extended address parameters in network functions.Bug Fixes
Translation