daniloc / PicoW_HomeAssistant_Starter

Everything you need to get started with your own Intranet of Things, using the high-quality, low-cost Pico W as the backbone.
MIT License
198 stars 20 forks source link

HA MQTT Integration only discovers a single device. #13

Open JacobChrist opened 11 months ago

JacobChrist commented 11 months ago

So I 'm not sure where the issues is here but I'm trying to connect two Pico Pi MQTT devices to HA. I can see that both are reporting unique ID's to HA's broker but only a single device is being discovered in HA. Though I don't think they need it, they both have unique names but otherwise the firmware is probably identical.

I belive the four clients are: 1: HA's MQTT Broker, 2: MQTT Explorer, 3: Pico Pi, 4: Pico Pi image

And in HA I see this: image

And indeed the device looks correct: image

I feel like this is an issue with mosquitto but it could be due to a discovery issues with this project or AHA.

daniloc commented 11 months ago

Upstream issue. You can find a workaround here:

https://github.com/dawidchyrzynski/arduino-home-assistant/issues/183

tl;dr: arduino-home-assistant doesn't unique the entities in its 2.0 release; use my fork that does instead.

JacobChrist commented 11 months ago

@daniloc Thanks for this, I've merged in your changes but it doesn't seem to have resolved the issue.

daniloc commented 11 months ago

You'll probably need to delete the device in Home Assistant and re-add it. Try that out and let me know how it goes.

JacobChrist commented 11 months ago

Thanks for the help BTW...

I've deleted the homeassistant and aha nodes in MQTT Explorer which causes the 1 device and 11 entities to disappear from HA.

It looks like your trying to create a uniqueId based on the mainUniqueId and uniqueIdSuffix. This id should have an "" between the mainUniqueId and uniqueIdSuffix. I'm not seeing any id's with underscores between them in MQTT Explorer.

This picture was taken after the old nodes have been deleted and uploading new code to the boards: image

JacobChrist commented 11 months ago

I think I found the issue here. If when the HADevice object is instantiated you pass in a uniqueId then this bit of code works and the entity uniqueId is prepended with the mainUniqueId. However, if you use setUniqueId after the device object has been instantiated this does not work.

Here is json string when creating the device using: `HADevice device("myUniqueId")

{
   "name":"FlowBot LED",
   "uniq_id":"myUniqueID_led","ic":"mdi:led-outline",
   "dev":{
      "ids":"myUniqueID",
      "name":"FlowBot by PONTECH","sw":"0.1"
   },
   "stat_t":"aha/myUniqueID/myUniqueID_led/stat_t",
   "cmd_t":"aha/myUniqueID/myUniqueID_led/cmd_t"
}

I'm pretty sure this is happening because the HASwitch instances I'm creating are globals and since they inherent from HABaseDeviceType and since this _uniqueId has not been set yet it defaults to using just the HASwitch uniqueId.

JacobChrist commented 11 months ago

I have confirmed that this is the issue, I now have two devices and 22 entities in HA. I have to manually created the uniqueId passed when the HADevice object is instantiated. This is sufficient for now, but there should probably be a ways to either:

  1. Pass the mac address in to the HADevice object when declared globally.
  2. Have all the entity uniqueId's update when a setUniqueId call is made before mqtt.begin().
daniloc commented 11 months ago

Iiiinteresting, and annoying. Come to think of it, I vaguely remember encountering this on ThermTerm, and you can see evidence of it in the weirdly complex constructor for HAInterface I ended up with:

HAInterface::HAInterface() : macAddress_(generateMacAddress()),
                             haDevice_(macAddress_.c_str()),
                             mqtt_(client, haDevice_),
                             temperatureSensor_("temperature"),
                             humiditySensor_("relative-humidity"),
                             lightSensor_("light-intensity"),
                             alertTrigger_("alert-trigger"),
                             hvacDevice_(
                                 "heat-pump",
                                 HAHVAC::TargetTemperatureFeature |
                                     HAHVAC::PowerFeature |
                                     HAHVAC::ModesFeature |
                                     HAHVAC::FanFeature)
{
}

The inside baseball here is that I built this starter before a-h-a 2.0 dropped and it seems to be a substantially ground-up rewrite where some of these small side effects haven't been caught. It fixed other bugs, though, and added deeper functionality so I think it's net positive despite this.

I would be curious if this particular failing is a result of my hacky fork though.

JacobChrist commented 11 months ago

I'm made a total bodge fix so I don't have to manually type in mac address when the HADevice is instantiated. Here is my solution:

  1. I turned my HADevice, HAMqtt and HASwitch into global pointers.
HADevice* pDevice;
HAMqtt* pMqtt;
HASwitch* pLed;
  1. I build a uniqueIdStr then create my objects and updated my global pointers like this:
char uniqueIdStr[30];
void HAIntegration::configure() {
    //Set device ID as MAC address
    byte mac[WL_MAC_ADDR_LENGTH];
    WiFi.macAddress(mac);
    snprintf(uniqueIdStr, 20, "HALed-%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5], mac[6]);
    Serial.println(uniqueIdStr);

    pDevice = new HADevice(uniqueIdStr); 
    pMqtt = new HAMqtt(client, *pDevice);

    static HASwitch led("LED"); // unique identifier must not contain spaces

    pLed = &led;
  1. If I need to access my objects outside of HAIntegration.cpp I do it like this:
    pLed->getCurrentState()

Don't forget to add the pointers to the HAIntegration.h file as externs.

here is a published message:

{"name":"HALed LED","uniq_id":"HALed-28cdc10f56bb_LED","ic":"mdi:led-outline","dev":{"ids":"HALed-28cdc10f56bb","name":"HALed","sw":"0.1","mf":"PONTECH.com","mdl":"HALED100"},"avty_t":"plp/HALed-28cdc10f56bb/avty_t","stat_t":"plp/HALed-28cdc10f56bb/HALed-28cdc10f56bb_LED/stat_t","cmd_t":"plp/HALed-28cdc10f56bb/HALed-28cdc10f56bb_LED/cmd_t"}

I'm not sure I would recommend this for human consumption but it might be worth putting on a branch for those that are not faint of heart.

JacobChrist commented 11 months ago

Ideally you should be able to create all the objects and update the uniqueId's before use and avoid the use of pointers.

JacobChrist commented 11 months ago

BTW, I have five Pico Pi's talking to HA and updating the mac address before I uploaded to a board was getting annoying and difficult of keep track of.