Skip to content

BREAKING CHANGE: Fix: use uniqueId for battery HA auto-discovery #2442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AndreasBoehm wants to merge 1 commit into
base: development
Choose a base branch
from
Open

BREAKING CHANGE: Fix: use uniqueId for battery HA auto-discovery #2442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/battery/HassIntegration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ class HassIntegration {

private:
static String sanitizeUniqueId(const char* value);
static String createBatteryId();

String _serial = "0001"; // pseudo-serial, can be replaced in future with real serialnumber
std::shared_ptr<Stats> _spStats = nullptr;

bool _publishSensors = true;
Expand Down
20 changes: 9 additions & 11 deletions src/battery/HassIntegration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,17 @@ void HassIntegration::publishSensor(const char* caption, const char* icon,
{
String sensorId = sanitizeUniqueId(caption);

String configTopic = "sensor/dtu_battery_" + _serial
String configTopic = "sensor/" + createBatteryId()
+ "/" + sensorId
Comment on lines +54 to 55
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Home Assistant MQTT discovery, what happens when an existing entity's uniq_id changes after a firmware update? Are old entities/devices automatically migrated or left as stale registry entries?

💡 Result:

Web search failed: Server error: no LLM provider could handle the message

🏁 Script executed:

# First, find and examine the HassIntegration.cpp file
fd -t f "HassIntegration.cpp" --exec wc -l {} \;

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 517

🏁 Script executed:

# Get the specific lines mentioned in the review
cat -n src/battery/HassIntegration.cpp | sed -n '50,170p'

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 4165

🌐 Web query:

Home Assistant MQTT discovery uniq_id change migration behavior

💡 Result:

  • unique_id is the identity key. Home Assistant uses an entity’s unique_id to tie MQTT-discovered entities to the Entity Registry (so you can later rename the entity_id and keep that rename). (home-assistant.io)
  • If you change the unique_id, Home Assistant will treat it as a different entity. Practically, that means you’ll get a new entity in HA, and the old one will remain as an orphan (often unavailable) until you remove it; HA will not automatically “migrate” the old entity’s entity_id, area, history settings, automations, etc., to the new unique_id. (This follows from how discovery creates/updates entities keyed by unique_id, and that removals are explicit.) (home-assistant.io)
  • Changing the discovery topic’s <object_id> is not the same as changing unique_id. The <object_id> segment in the discovery topic does not control the resulting entity_id; best practice is to set that <object_id> to the unique_id (so you don’t accidentally create parallel entities when you change topics). (home-assistant.io)
  • How to clean up the old entity (recommended “migration” pattern):
    1. Publish the new discovery config (with the new unique_id) so the new entity appears.
    2. Publish an empty (retained) payload to the old discovery .../config topic to delete the old discovered entity and clear the retained config. (home-assistant.io)
  • Related but different: {"migrate_discovery": true} is a special mechanism for migrating single-component discovery topics to device discovery while retaining settings; it’s not a general “unique_id change migration.” (home-assistant.io)

Citations:

🌐 Web query:

Home Assistant MQTT entity unique ID change upgrade retained topics

💡 Result:

Web search failed: Server error: no LLM provider could handle the message

🏁 Script executed:

# Search for migration or cleanup related code
rg -i "migration|cleanup|legacy|orphan" src/battery/ --type cpp --type h

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 182

🏁 Script executed:

# Check for CHANGELOG or release notes mentioning battery IDs or migration
fd -t f "CHANGELOG|RELEASE|NEWS|MIGRATION" --iglob "*" | head -20

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 271

🏁 Script executed:

# Search for any cleanup of old discovery topics in the codebase
rg "publish.*config.*empty\|empty.*payload\|retained" src/ --type cpp

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 51

🏁 Script executed:

# Check git history for when this change was made and if there were migration notes
git log --oneline -20 src/battery/HassIntegration.cpp 2>/dev/null || echo "Git history not available"

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 125

🏁 Script executed:

# Get the full commit details to see what changed
git show dcb983d8 --no-patch

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 413

🏁 Script executed:

# Check what the old battery ID format was
git show dcb983d8:src/battery/HassIntegration.cpp 2>/dev/null | head -100

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 2954

🏁 Script executed:

# Verify cpplint compliance for the affected file
# Check if cpplint is available
which cpplint || echo "cpplint not available"

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 151

🏁 Script executed:

# At minimum, check for obvious style issues in the affected lines
cat -n src/battery/HassIntegration.cpp | sed -n '54,56p; 64p; 108,110p; 118p; 146p'

Repository: hoylabs/OpenDTU-OnBattery

Length of output: 495

Add migration documentation for Home Assistant discovery identity changes.

The battery discovery uniq_id (lines 64, 118) and device ids (line 146) now use createBatteryId() to fix multi-DTU collisions. However, on upgrades, Home Assistant will treat these as new entities and leave old ones orphaned—automations and dashboards bound to the old IDs will break, and HA does not auto-migrate them.

To mitigate this for users, add release notes documenting:

  1. The change reason (fixing hardcoded '0001' collisions in multi-DTU setups)
  2. Expected behavior on upgrade (new entities, old ones become unavailable)
  3. Cleanup procedure: publish an empty retained payload to the old discovery topics (e.g., homeassistant/sensor/0001/<sensorId>/config) to explicitly remove orphaned entities from the HA registry

Alternatively, consider a one-time cleanup routine on first startup after the ID changes to publish empty payloads for any legacy discovery topics.

Also applies to: 54–55, 108–110

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/battery/HassIntegration.cpp` around lines 54 - 55, The Home Assistant
discovery IDs were changed to use createBatteryId(), which will make HA treat
entities as new and leave old ones orphaned; add migration documentation to
release notes explaining the reason (fixing hardcoded '0001' collisions),
expected upgrade behavior (new entities, old ones unavailable), and explicit
cleanup steps (publish an empty retained payload to legacy discovery topics such
as homeassistant/sensor/0001/<sensorId>/config to remove orphaned entities).
Also implement an optional one-time migration routine in HassIntegration (on
first startup after upgrade) that iterates legacy discovery topic patterns
(e.g., sensor/0001/<sensorId>/config) and publishes empty retained payloads to
remove old entities, ensuring it only runs once and records completion to avoid
repeated runs; reference createBatteryId(), configTopic, uniq_id and device ids
when locating the related code to update.

+ "/config";

String statTopic = MqttSettings.getPrefix() + "battery/";
// omit serial to avoid a breaking change
// statTopic.concat(_serial);
// statTopic.concat("/");
statTopic.concat(subTopic);

JsonDocument root;
root["name"] = caption;
root["stat_t"] = statTopic;
root["uniq_id"] = _serial + "_" + sensorId;
root["uniq_id"] = createBatteryId() + "_" + sensorId;

if (icon != NULL) {
root["icon"] = icon;
Expand Down Expand Up @@ -108,20 +105,17 @@ void HassIntegration::publishBinarySensor(const char* caption,
{
String sensorId = sanitizeUniqueId(caption);

String configTopic = "binary_sensor/dtu_battery_" + _serial
String configTopic = "binary_sensor/" + createBatteryId()
+ "/" + sensorId
+ "/config";

String statTopic = MqttSettings.getPrefix() + "battery/";
// omit serial to avoid a breaking change
// statTopic.concat(_serial);
// statTopic.concat("/");
statTopic.concat(subTopic);

JsonDocument root;

root["name"] = caption;
root["uniq_id"] = _serial + "_" + sensorId;
root["uniq_id"] = createBatteryId() + "_" + sensorId;
root["stat_t"] = statTopic;
root["pl_on"] = payload_on;
root["pl_off"] = payload_off;
Expand Down Expand Up @@ -149,7 +143,7 @@ void HassIntegration::publishBinarySensor(const char* caption,
void HassIntegration::createDeviceInfo(JsonObject& object) const
{
object["name"] = *_spStats->getHassDeviceName();
object["ids"] = _serial;
object["ids"] = createBatteryId();
object["cu"] = MqttHandleHass.getDtuUrl();
object["mf"] = "OpenDTU";
object["mdl"] = *_spStats->getManufacturer();
Expand All @@ -164,6 +158,10 @@ void HassIntegration::publish(const String& subtopic, const String& payload) con
MqttSettings.publishGeneric(topic.c_str(), payload.c_str(), Configuration.get().Mqtt.Hass.Retain);
}

String HassIntegration::createBatteryId() {
return MqttHandleHass.getDtuUniqueId() + "_battery";
}

String HassIntegration::sanitizeUniqueId(const char* value) {
String sensorId = value;

Expand Down
Loading