Skip to content

Comments

Fix channel type creating#55

Merged
seime merged 1 commit intoseime:masterfrom
CyborgAndy:fix-channel-type
Oct 3, 2025
Merged

Fix channel type creating#55
seime merged 1 commit intoseime:masterfrom
CyborgAndy:fix-channel-type

Conversation

@CyborgAndy
Copy link
Contributor

  1. Fix resolving deviceСlass - always returned the defaultDeviceClass
  2. Fix api.proto - uniqueId is deprecated and was removed in the v36.0.0 aioesphomeapi
  3. And as a result of point 2 - сhange ChannelTypeUID naming to next format: ThingId_ObjectId

Signed-off-by: CyborgAndy <cyborg.andy@gmail.com>
@seime seime requested a review from Copilot October 3, 2025 10:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes several issues related to channel type creation and device class resolution in the ESPHome binding. The main purpose is to align the codebase with the deprecation of the uniqueId field in aioesphomeapi v36.0.0 and fix a bug in device class resolution.

  • Removed deprecated unique_id field from protobuf definitions and replaced with reserved field markers
  • Fixed device class resolution bug that always returned the default device class
  • Updated channel type UID naming format from using uniqueId to ThingId_ObjectId pattern

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

File Description
src/main/proto/api.proto Deprecated unique_id fields across all entity response messages
src/main/java/no/seime/openhab/binding/esphome/internal/message/AbstractMessageHandler.java Fixed device class resolution bug and updated channel type UID format
Multiple message handler files Replaced getUniqueId() calls with getObjectId() for channel type creation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@seime
Copy link
Owner

seime commented Oct 3, 2025

Thanks for your contribution @CyborgAndy. Question; have you considered whether changing the channel type id might break existing installation with managed things/items?

@CyborgAndy
Copy link
Contributor Author

CyborgAndy commented Oct 3, 2025

Hi @seime
I just started using your binding with the latest version of the ESPHome, so I don't have a way to test this.
I don't think this will break existing things. After all, the logic of the binding is such that when you enable/disable thing or update the binding, the channel types are recreated every time.
Will the proposed ChanelTypeID format really be unique within the binding? I think so. Maybe you have other suggestions.

@seime
Copy link
Owner

seime commented Oct 3, 2025

A simple test would be to use the binding as is, discover an esp and add it via the UI (and not file). Then replace the binding with this new version, and check that channels still are present and working.

You are raising another question - will it be unique accross multiple devices with the same entityId? If the thing id is still used as part of the channel type ui, they will be unique.

@CyborgAndy
Copy link
Contributor Author

Okay, I'll use the version of the ESPHome where the uniqueId is still present and check how it works.

@ccutrer
Copy link
Contributor

ccutrer commented Oct 3, 2025

Changing channel type UIDs will not impact any existing linked items. The Home Assistant (MQTT) binding has changed channel type UIDs a couple times with no issues.

@ccutrer
Copy link
Contributor

ccutrer commented Oct 3, 2025

Confirmed - I just compiled with this commit and put it in my production system, and existing linked item states are still updating just fine.

@seime seime merged commit 25ed483 into seime:master Oct 3, 2025
2 checks passed
@CyborgAndy CyborgAndy deleted the fix-channel-type branch October 4, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants