NimmLor / esp8266-fastled-iot-webserver

A universal software for all my LED projects, with many awesome features
https://www.thingiverse.com/surrbradl08/designs
GNU General Public License v3.0
366 stars 94 forks source link

HomeAssistant not recognizing separate esp boards #148

Closed BakersChoice closed 3 years ago

BakersChoice commented 3 years ago

Hello,

I'm trying to add multiple esp boards to my HomeAssistant set up via MQTT and am having some weird results. It will recognize the device, but not the separate entities, i.e., each esp board. When I set up multiple boards, give them different MQTT topics, and push the changes, HomeAssistant doesn't recognize them as separate. Rather one button controls 2, and if I add a 3rd, then it just controls 2 at a time, always relinquishing control of 1. I've tried to look at the MQTT Topic in the HomeAssistant log and it only shows a state of "on" but is unresponsive to toggling it off and on.

WarDrake commented 3 years ago

Can you leave a copy your config changes here to examine? use an MQTT explorer and check that the different topics are indeed being created, are all esps configured as the same device type or different ones?

bb-Ricardo commented 3 years ago

Is it important that the device name is different?

WarDrake commented 3 years ago

Not necessarily, what's important is that the topic is different, but the device name is part of the topic. if the devices have the same name, it's possible they're reading the same topic and that's why they're reacting together.

however looking at the MQTT explorer will confirm whether that's the issue or it's something else.

BakersChoice commented 3 years ago

It looks like the different topics are not being created. I have the topics set as "homeassistant/light/lantern1" and it goes from "/lantern1" to "/lantern3". I also changed the device name for each to correspond with their topic, so lantern1 through lantern3. Looking at my MQTT server, it's reading a topic called "homeassistant/lantern/lantern", which was an old one I set it to. To me it appears the webUI is not updating the MQTT settings.

WarDrake commented 3 years ago

I see what the Issue is, Topic subcription is not based on the Config manager, but on the hardcoded values on the script, so the esp post to the correct topic, but subscribes and reads it's data from the base one.

I'll push a fix in a little while.

WarDrake commented 3 years ago

This commit should solve the Issue bc3f8f3cad2a5b2c40006ee57f54895127ae7a7a
it now follows the web config when making itself known to homeassistant and when publishing and reading states.

can you please check and confirm @BakersChoice

BakersChoice commented 3 years ago

It's coming up with a few errors now, here's what Arduino IDE is telling me:

In function 'void loop()':

esp8266-fastled-iot-webserver:267:32: error: expected ')' before string constant

     #define MQTT_TOPIC_SET "/set"                                   // MQTT Topic to subscribe to for changes(Home Assistant)

                            ^

in expansion of macro 'MQTT_TOPIC_SET'

             mqttClient.subscribe(cfg.MQTTTopic MQTT_TOPIC_SET);

                                                ^

esp8266-fastled-iot-webserver:1360:59: error: expected ')' before string constant

             if (mqttClient.beginPublish(cfg.MQTTTopic "/config", n, true) == true) {

                                                       ^

exit status 1

expected ')' before string constant

This report would have more information with "Show verbose output during compilation" option enabled in File -> Preferences.

WarDrake commented 3 years ago

Hmm... that's odd, it compiles correctly here for me, what's your build environment?

image

BakersChoice commented 3 years ago

I'm in Arduino IDE. This is the section of code it's having issues with.

"if (mqttClient.beginPublish(cfg.MQTTTopic "/config", n, true) == true {"

WarDrake commented 3 years ago

Yeah, I realized I made a mistake and compiled with MQTT off, it is fixed now, let me know if it works this time around.

BakersChoice commented 3 years ago

image

So here's the view from my MQTT Explorer. I'm not sure why but it keeps repeating itself like that down the line.

WarDrake commented 3 years ago

I suppose that because I got clever with my the constant/string concat and it failed to work properly even tho it compiles was trying to avoid making the set parameter a public var, but I'm just gonna do that, it'll take care of it

I'll have a fix ready in a couple hours.

BakersChoice commented 3 years ago

No worries thank you for all your help with this!

WarDrake commented 3 years ago

Can you please test this branch commit 7882c84d09f13fbc28de93d1b9abaf57ca7fffe4
I don't have an esp handy to test myself, delete the topmost lantern1 topic right underneath the first light, and see if this one creates the topics normally.
the MQTT set topic is added to the config, you'll need to upload the data folder again, the set topic should be : /set

BakersChoice commented 3 years ago

Sounds good I'll give it a shot

BakersChoice commented 3 years ago

So the device is coming up on my MQTT server just fine now, and I've been able to change its settings in the UI and it's updating okay it looks like. The only issue I'm having now is that HomeAssistant isn't discovering it as a device.

WarDrake commented 3 years ago

Can you use the MQTT explorer and show me the contents of the topic : homeassistant/light/lantern1/config the problem should be there.

BakersChoice commented 3 years ago

Here's what that looks like.

image

WarDrake commented 3 years ago

Yeah, that's the Issue right there, You have the state instead of the config in that topic, I just pushed another commit to that same branch for you to test, sorry I can't really test it myself right now. c5fd6bdb8cb043b936f08e16216b62dd202d389d

2 things. 1, the serial output has some new debug messages I'll need those please 2, turn the esp off, delete the config topic, and with the MQTT Explorer open, power the esp on, on the right side, you should get a history of the config topic after it's created again, post that over here too please, need to check if config is set properly and then overwritten or if it's just set incorrectly right now.

Thanks

BakersChoice commented 3 years ago

I think this is the debug messages for the serial output but let me know if I got this wrong:

Configuration Publishing Begun Configuration Topic :/config Configuration Sent Configuration Publishing Finished Status Topic :homeassistant/light/lantern2 Sending Initial Status Status Topic :homeassistant/light/lantern2 Status Topic :homeassistant/light/lantern2 Status Topic :homeassistant/light/lantern2 Status Topic :homeassistant/light/lantern2

Here is the current config topic:

{
   "~":"homeassistant/light/lantern2",
   "name":"Lantern2",
   "dev":{
      "ids":"8C:AA:B5:1B:11:31",
      "mf":"Surrbradl08",
      "mdl":"0.4.4",
      "name":"Lantern2"
   },
   "stat_t":"~",
   "cmd_t":"/set",
   "brightness":true,
   "rgb":true,
   "effect":true,
   "uniq_id":"8C:AA:B5:1B:11:31",
   "schema":"json",
   "effect_list":[
      "Pride",
      "Color Waves",
      "Horizontal Rainbow",
      "Solid Rainbow",
      "Confetti",
      "Sinelon",
      "Beat",
      "Juggle",
      "Fire",
      "Water",
      "Strobe",
      "Rainbow Strobe",
      "Smooth Rainbow Strobe",
      "Rainbow Roll",
      "Rainbow Beat",
      "Palette Fades",
      "Rainbow Chase",
      "Rainbow Dots",
      "Rainbow Fades",
      "Police Lights",
      "Glitter",
      "Snow Flakes",
      "Lightning",
      "Rainbow Twinkles",
      "Snow Twinkles",
      "Cloud Twinkles",
      "Incandescent Twinkles",
      "Retro C9 Twinkles",
      "Red & White Twinkles",
      "Blue & White Twinkles",
      "Red, Green & White Twinkles",
      "Fairy Light Twinkles",
      "Snow 2 Twinkles",
      "Holly Twinkles",
      "Ice Twinkles",
      "Party Twinkles",
      "Forest Twinkles",
      "Lava Twinkles",
      "Fire Twinkles",
      "Cloud 2 Twinkles",
      "Ocean Twinkles",
      "Solid Volume Visualizer",
      "Static Rainbow Volume Visualizer",
      "Flowing Rainbow Volume Visualizer",
      "Tri-Color Volume Visualizer",
      "Wave Visualizer",
      "Center Visualizer",
      "Solid-Color Pair Bullet Visualizer",
      "Solid-Color Complementary Bullet Visualizer",
      "Blue/Purple Bullet Visualizer",
      "Beat-Bullet Visualization",
      "Bass Ring Visualizer",
      "Kick Ring Visualizer",
      "Rainbow Band Visualizer",
      "Single Color Band Visualizer",
      "Solid Color"
   ]
}
WarDrake commented 3 years ago

Got an ESP to test it, so this is tested working now, let me know if you can test it too and if you run into any more issues

BakersChoice commented 3 years ago

Just tested it and it works on my end too, thank you so much for your help!

bb-Ricardo commented 3 years ago

Hi @WarDrake and @BakersChoice,

i merged the changes back into the "in-development" branch to tie it all together. I changes some bits around and was wondering if you can run a final test again.

Here I changed the size of the char array to fit full size of cfg.MQTTTopic and cfg.MQTTSetTopic. https://github.com/NimmLor/esp8266-fastled-iot-webserver/blob/54561ac1ce2276b7dc7145795575c30c6a92b49f/esp8266-fastled-iot-webserver.ino#L1403-L1410

Same here: https://github.com/NimmLor/esp8266-fastled-iot-webserver/blob/54561ac1ce2276b7dc7145795575c30c6a92b49f/esp8266-fastled-iot-webserver.ino#L1432-L1434

Also is this publish call with just cfg.MQTTTopic correct? Above we appended config and set topic. https://github.com/NimmLor/esp8266-fastled-iot-webserver/blob/54561ac1ce2276b7dc7145795575c30c6a92b49f/esp8266-fastled-iot-webserver.ino#L4899

I'm not familiar with MQTT that's the reason for this probably stupid question.

WarDrake commented 3 years ago

Just tested it, it works fine.

The publish line is publishing a status to the base topic, config one is to provide autodiscovery for homeassistant and set is to receive state changes

so yes, publish is only to the base topic.

bb-Ricardo commented 3 years ago

Hi @WarDrake,

thank you for testing. I still try to understand why appending "/set" to the topic wouldn't be enough. The same way as "/config" works. Why would you need to change it? Isn't the main topic the distinctive element?

I'm asking all this silly questions to find out if it's really necessary to have another config option which would also just be fine if hardcoded, just like "/config".

thank you.

WarDrake commented 3 years ago

That's because you can choose your own set topic, /set is the default in homeassistant, but you can make it anything you want, and we send that over to the config, but some other mqtt configurations might use a different default and you can config that there.

/config is a fixed one because it generates a homeassistat autodiscovery specific config, so that one is a fixed one.

bb-Ricardo commented 3 years ago

great thank you