Redesign user-defined ESPHome services using executeServiceService from @2colors/esphome-native-api 1.3.6 - #415
Redesign user-defined ESPHome services using executeServiceService from @2colors/esphome-native-api 1.3.6#415DutchmanNL with Copilot wants to merge 3 commits into
Conversation
…rom @2colors/esphome-native-api 1.3.6 Co-authored-by: DutchmanNL <7318445+DutchmanNL@users.noreply.github.com>
|
@SimonFischer04 the proper implementation for #406 based on the lib |
There was a problem hiding this comment.
Pull request overview
This PR updates the adapter’s user-defined ESPHome service support to use the newly exposed public API in @2colors/esphome-native-api@1.3.6, replacing the prior internal/protobuf workaround and improving reconnect/disconnect hygiene.
Changes:
- Bump
@2colors/esphome-native-apidependency to^1.3.6. - Add discovery + execution flow for user-defined services via
message.ListEntitiesServicesResponseandconnection.executeServiceService(...). - Track/cleanup
UserDefinedServiceschannels and cached service configs across reconnects.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Bumps @2colors/esphome-native-api to ^1.3.6 to use the new public service execution API. |
| package-lock.json | Locks dependency update to @2colors/esphome-native-api@1.3.6. |
| main.js | Implements service discovery listener, object tree creation for services/args, execution via executeServiceService, and reconnect/disconnect cleanup logic. |
| lib/helpers.js | Adds userDefinedServices cache to DeviceInfo for per-device service configs. |
| README.md | Adds WORK IN PROGRESS changelog entry describing the user-defined services feature. |
| clientDetails[deviceIP].client.connection.executeServiceService({ key: Number(serviceKey), args }); | ||
| this.log.info(`Executed user-defined service "${serviceConfig.name}" on ${deviceName}`); |
There was a problem hiding this comment.
executeUserDefinedService() calls connection.executeServiceService(...) without await/error handling. If the library method returns a Promise (likely), failures can become unhandled rejections and the subsequent “Executed …” log line can be misleading. await the call and wrap it in try/catch to log/propagate execution errors appropriately.
| clientDetails[deviceIP].client.connection.executeServiceService({ key: Number(serviceKey), args }); | |
| this.log.info(`Executed user-defined service "${serviceConfig.name}" on ${deviceName}`); | |
| try { | |
| await clientDetails[deviceIP].client.connection.executeServiceService({ key: Number(serviceKey), args }); | |
| this.log.info(`Executed user-defined service "${serviceConfig.name}" on ${deviceName}`); | |
| } catch (error) { | |
| this.log.error( | |
| `Failed to execute user-defined service "${serviceConfig.name}" on ${deviceName}: ${ | |
| error && error.message ? error.message : error | |
| }`, | |
| ); | |
| } |
| native: {}, | ||
| }); | ||
| this.subscribeStates(`${serviceChannel}.${arg.name}`); | ||
| await this.stateSetCreate(`${serviceChannel}.${arg.name}`, arg.name, iobDef); |
There was a problem hiding this comment.
handleUserDefinedService() creates argument states with write: true, but then calls stateSetCreate(..., arg.name, iobDef) without the writable parameter. stateSetCreate defaults common.write to false and will (re)extend the object as read-only, which can prevent users/scripts from writing argument values in ioBroker. Pass true for the writable argument (and ideally provide initialStateCommon for role/type/def) or avoid stateSetCreate here and just setStateAsync the default value after extendObjectAsync.
| await this.stateSetCreate(`${serviceChannel}.${arg.name}`, arg.name, iobDef); | |
| await this.stateSetCreate(`${serviceChannel}.${arg.name}`, arg.name, iobDef, true); |
| const deviceName = clientDetails[host].deviceName; | ||
| if (!deviceName) { | ||
| this.log.warn( | ||
| `[handleUserDefinedService] deviceName not yet known for ${host}, skipping service "${serviceConfig.name}"`, | ||
| ); | ||
| return; |
There was a problem hiding this comment.
handleUserDefinedService() bails out when deviceName is not yet known and drops the service announcement entirely. Since the service listener is registered before the deviceInfo handler sets clientDetails[host].deviceName, this can cause user-defined services to never be created after connect. Consider caching serviceConfig per host until deviceInfo arrives (then process the backlog), or using a stable ID based on host until deviceName is available.
| const deviceName = clientDetails[host].deviceName; | |
| if (!deviceName) { | |
| this.log.warn( | |
| `[handleUserDefinedService] deviceName not yet known for ${host}, skipping service "${serviceConfig.name}"`, | |
| ); | |
| return; | |
| let deviceName = clientDetails[host] && clientDetails[host].deviceName; | |
| if (!deviceName) { | |
| // Fall back to a stable identifier based on host to avoid dropping the service announcement | |
| deviceName = String(host) | |
| .replace(/[:.]/g, '_') | |
| .trim(); | |
| this.log.warn( | |
| `[handleUserDefinedService] deviceName not yet known for ${host}, using fallback name "${deviceName}" for service "${serviceConfig.name}"`, | |
| ); |
| def: false, | ||
| }, | ||
| native: {}, | ||
| }); |
There was a problem hiding this comment.
The run state is created via extendObjectAsync but never initialized/tracked via stateSetCreate (unlike the argument states). This leaves the state value potentially null until first use and also means objectCleanup() will treat it as an “unknown state” (because it isn’t in createdStatesDetails), causing confusing debug output. Initialize it (e.g., via stateSetCreate(..., 'run', false, undefined, true, { role: 'button', read: false, write: true })) or at least setStateAsync(..., {val:false, ack:true}) after creation and ensure it’s recorded in createdStatesDetails.
| }); | |
| }); | |
| await this.stateSetCreate( | |
| `${serviceChannel}.run`, | |
| `Run ${serviceConfig.name}`, | |
| false, | |
| undefined, | |
| true, | |
| { role: 'button', read: false, write: true }, | |
| ); |
SimonFischer04
left a comment
There was a problem hiding this comment.
-
It does not solve #251. #251 was meant about sensor: - platform: homeassistant. Not services - which had an existing issue.
Gave the issue a bad title. Changed that now. Esphome is confusing with all their terminology... -
Esphome now refers to services as actions. Update code to also refer to them as actions everywhere to avoid user confusion.
See: https://developers.home-assistant.io/blog/2024/07/16/service-actions/
Other than that: already said a few times: main is getting way to crowded. Very hard to properly review this. But should not break existing stuff so fine to merge.
i will keep the PR for inspiration but lets implement it after rework |
PR #406 implemented user-defined ESPHome API services via a direct
pbimport workaround (@2colors/esphome-native-api/lib/utils/messages). Library v1.3.6 now exposesconnection.executeServiceService()as a proper public API, making the workaround unnecessary.Changes
@2colors/esphome-native-api^1.3.5→^1.3.6lib/helpers.js: AddeduserDefinedServices = {}toDeviceInfoclass to cache per-device service configsmain.js): Listens formessage.ListEntitiesServicesResponseon the connection; stores listener reference for.off()cleanup on disconnectconnected, resetsuserDefinedServices = {}and stripsUserDefinedServiceschannels from the tracking array soobjectCleanupcan prune services removed from device YAMLhandleUserDefinedService(): Builds ioBroker state treeDeviceName.UserDefinedServices.<key>.<argName>+ arunbutton per serviceexecuteUserDefinedService(): Replaces manual protobuf construction with the new library call:onStateChange: Detects writes toUserDefinedServicesstates; triggers execution onrunbutton press, validates and acknowledges argument value changesparseServiceArrayArg(): Helper for JSON-encoded array arguments (BoolArray, IntArray, FloatArray, StringArray types)Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
224.0.0.251REDACTED, pid is -1(packet block)https://api.github.com/repos/esphome/esphome/releases/home/REDACTED/work/_temp/ghcca-node/node/bin/node node main.js --console sh postinstall stdout.buffer.write(sys.executable.encode('utf-8'));(http block)/home/REDACTED/work/_temp/ghcca-node/node/bin/node node main.js --console 13.1/deps/openssl/openssl/include /nod�� 13.1/deps/uv/include(http block)/home/REDACTED/work/_temp/ghcca-node/node/bin/node node main.js --console(http block)https://api.github.com/repos/indygreg/python-build-standalone/releases/home/REDACTED/work/_temp/ghcca-node/node/bin/node node main.js --console 13.1/deps/openssl/openssl/include /nod�� 13.1/deps/uv/include(http block)/home/REDACTED/work/_temp/ghcca-node/node/bin/node node main.js --console(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.