search

Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
14.84k stars 3.2k forks source link

Change Proposal: Reorganizing MQTT Topics #207

Open timothybrown opened 5 years ago

timothybrown commented 5 years ago

Okay, so I'm currently working on rewriting the HA Discovery code from the ground up. Part of this requires me to basically redo the MQTT topics. Right now it's kind of a mess and none of the topics are ideal for Home Assistant (or other home automation platforms).

The first step is to change the topic names and their payload. Here is what I've come up with:

This puts the topic names in-line with other projects (Espurna, ESPHome, etc.) and makes it easier to integrate into custom code and various home automation platforms without resorting to complex templating or XML decoding (however, because we've retained the API topic, this can still be done for advanced users).

Judging by a look through the issue list, it appears most people are using MQTT for HA Discovery support, which means this change will be transparent for those users (i.e., nothing will break for them and it will just work). This will only be a breaking change for those who manually configure HA, use other home automation platforms or use MQTT as a transport for custom software. I feel this is a very small percentage of use cases and this change will ultimately make it easier to use for them.

I'm interested to hear from anyone using the MQTT support in a non-HA setting (i.e., with custom code, other automation platforms and so forth). Would these topics be sufficient for your use? Is there anything else you'd like to see?

Comments from others (contributors, users and @Aircoookie alike) are welcomed as well!

I've currently got the basics of this implemented in my development fork (not yet published) and it's working well, but I wanted to get a discussion going before I go any further with it.

Edit: Renamed the power topic to switch and added availability to the list of topics. (When the device connects to the broker it will set the status topic to online then register what’s called a “last will”, which allows the broker to take care of setting the status topic to offline for us when the device disconnects.)

timothybrown commented 5 years ago

I should also add that, because of the way I've structured my rewrite, when @Aircoookie finishes support for segments, I'll be able to add segment support into the MQTT code as well. What this means is you could have each segment show up as a separate light in Home Assistant, or you could easily control each segment through MQTT using your custom code. The idea is, each segment would append a number onto the base device topic:

Device Topic/0/[switch|color|brightness|fx|api|status] Device Topic/0/[switch|color|brightness|fx|api]/set Device Topic/1/[switch|color|brightness|fx|api|status] Device Topic/1/[switch|color|brightness|fx|api]/set Device Topic/2/[switch|color|brightness|fx|api|status] Device Topic/2/[switch|color|brightness|fx|api]/set

This has a lot of possibilities!

brentbrooks70 commented 5 years ago

@timothybrown This would ease my implementation of segment control from the backend platform(QIOT Suite) I am using greatly going forward. I think it would actually make the entire system easier to work with via MQTT.

  1. In the power. or now switch, command should there be a 2 (toggle) option? 1a. I guess you could always use the Device Topic/api/set with a T2 so maybe not?
  2. I know this `might be a little off-topic (maybe needs its own issue) but is there any possibility of exposing the COOLING setting via MQTT (or HTTP API or both). I know this is only used by one FX but I have 2 lights that use the fire_2012 FX as the norm and I really miss not having that setting in WLED. I don't care if it is exposed on the GUI or not but, I would even prefer it to the speed control for that FX
  3. Again, this could be done Device Topic/api/set FP but exposing palette control may not be bad either
  4. My only other thought would be (again probably needs an issue of its own) to change responses from XML to JSON, I'm having difficulty in getting the MQTT messages parsed properly in QIOT Suite as it always expects JSON for some stupid reason.

Sorry if it seems like I'm rambling, sort of thinking (out loud) in writing.

Thanks for your work on this and asking for opinions before just making changes!!

Aircoookie commented 5 years ago

Hi, thank you for ewriting the MQTT integration! I absolutely agree, the topics I chose were quite arbitrary. This is often a problem when implementing something you are not an expert with. I definitely agree with bringing the implementation in line with other projects and easing the integration into HA and similar services, so I am very grateful for your help! Your choice of topics seems to make sense for me, this is a lot more transparent and easy to use than the current system. Maybe we can add speed, intensity and palette, but that could also be done at a later stage.

In fact I haven't been in contact with anyone who used MQTT outside of the home automation use case. HA is by far the most common, but there are users of openHAB and domoticz. Of course I will notify them about the breaking change, there is a high chance that your proposal will benefit the complexity of their implementation as well though!

About segments, the ESP basics are already implemented, just the Javascript UI and a few finishing touches are missing. I like your proposal, structuring the topics like that is very logical! You can take a look at wled19_json.ino to get an idea how to access them. Basically, you can use the strip.getSegment(index) method to access the Segment data object, which contains all the important variables of each segment:

typedef struct Segment { // 21 bytes
      uint16_t start;
      uint16_t stop; //segment invalid if stop == 0
      uint8_t speed;
      uint8_t intensity;
      uint8_t palette;
      uint8_t mode;
      uint8_t options; //bit pattern: msb first: transitional tbd tbd tbd tbd paused reverse selected
      uint32_t colors[NUM_COLORS];
      //...
      bool isActive()
      {
        return stop > start;
      }
      uint16_t length()
      {
        return stop - start;
      }
    } segment;

strip.getMaxSegments() will be helpful, too. If you want to support setting segment bounds via MQTT (i'm not sure whether that would make sense), use strip.setSegment(index, start, stop). Let me know if there is anything you need, I'm open to changes to the internal Segment api!

A consideration to make with segment support in general is which segment's values to return via the default APIs (XML and MQTT without segment number in topic). I don't think that returning the first segment would make sense for all users, so I'm planning to make this "return segment" user configurable. It is hardcoded to 0 right now, but you can already use strip.getReturnedSegmentId()

I have two small questions:

Thanks once again! Looking forward to hearing from you and let me know if there are any issues!

brentbrooks70 commented 5 years ago

What about the group topics (like /wled/all)? HA probably won't use them (because I assume there are better ways to group devices). Do you think there are use cases for them or should we drop them completely? If we keep them, they should of course mirror the device topic names, but only subscribe to the /set topics, not publish anything.

@Aircoookie - IMHO you could drop the ALL, topic, every HA setup (including my own) has grouping abilities or if you really needed groups couldn't you configure a UDP port per group and the UDP sync would handle it for you?

timothybrown commented 5 years ago

@Aircoookie @brentbrooks70 I’ll respond to the other parts of both your posts later tonight when I get more time. I just wanted to respond to the MQTT group stuff right now while I’m thinking about it.

Yes, I kind of feel that MQTT grouping could be dropped without affecting anyone. As @brentbrooks70 mentioned, we already have the built-in UDP sync functionality that does the same thing, plus most home automation platforms have the ability to group multiple devices into a single “virtual” device.

I’ll give you a good example: My bathroom vanity has three cheap Ai-Light RGBW bulbs installed (running Espurna). Now, I could use Espurna’s MQTT Group feature to have them all listen to the same topic and only add one of them to HA, but because they support HA Discovery, it’s easier for me to simply let HA find all three bulbs and create a light group that presents them as a single virtual bulb.

In fact, I’m hard pressed to think of a setup that UDP Sync or Light Groups wouldn’t cover for WLED. (But of course I don’t know every setup, so if anyone can think of one please share!)

aidbish commented 5 years ago

Updating the MQTT topics to work correctly will be a big win especially for those of us running HA. Also agree that the group topic can be removed without any if at all detriment as noted above most automation software allows you to create groups.

Happy to be a willing tester of the implementation

JAMESBOWLER commented 5 years ago

I argree its not too hard for me to change. I new around here and use openhab mqtt for control.

Looking at the HA docs for mqtt the JSON schema alows the most features to be used. https://www.home-assistant.io/components/light.mqtt/#json-schema

timothybrown commented 5 years ago

@brentbrooks70

@timothybrown This would ease my implementation of segment control from the backend platform(QIOT Suite) I am using greatly going forward. I think it would actually make the entire system easier to work with via MQTT.

Yes, that was my thought as well!

  1. In the power. or now switch, command should there be a 2 (toggle) option? 1a. I guess you could always use the Device Topic/api/set with a T2 so maybe not?

Yes, I could easily add a toggle command if it would be useful for you. I had thought about it, but left it out because I couldn’t immediately come up with a use case for it. (Most home automation software will simply track the state by subscribing to the power topic, or keep track of the state itself and work in a so called optimistic mode.)

  1. I know this `might be a little off-topic (maybe needs its own issue) but is there any possibility of exposing the COOLING setting via MQTT (or HTTP API or both). I know this is only used by one FX but I have 2 lights that use the fire_2012 FX as the norm and I really miss not having that setting in WLED. I don't care if it is exposed on the GUI or not but, I would even prefer it to the speed control for that FX

Please open an issue for this and I’ll look into adding it to the API! @Aircoookie may have some thoughts on it too.

  1. Again, this could be done Device Topic/api/set FP but exposing palette control may not be bad either

See my reply to @Aircoookie below.

  1. My only other thought would be (again probably needs an issue of its own) to change responses from XML to JSON, I'm having difficulty in getting the MQTT messages parsed properly in QIOT Suite as it always expects JSON for some stupid reason.

Yes, I planned on opening an issue about this as well! I’m new as a contributor to the project, so I wanted to get @Aircoookie’s thoughts on why he went with XML instead of JSON for responses. XML is a very verbose schema compared to JSON. We’ve already got the dependency baggage of the JSON library, so why not go all the way and have responses in JSON? It would use less memory, bandwidth and flash, plus it’s easier to encode/decode when writing custom software to control your devices.

My thought is that the JSON API is new and maybe his plan is to eventually switch over to it.

Sorry if it seems like I'm rambling, sort of thinking (out loud) in writing.

You’re not rambling at all. Personally, I like people thinking out loud like this in a development environment; it’s one of the things that make open source and communities like GitHub so great!

Thanks for your work on this and asking for opinions before just making changes!!

And thank you for the feedback, keep it coming!

@Aircoookie

Hi, thank you for ewriting the MQTT integration! I absolutely agree, the topics I chose were quite arbitrary. This is often a problem when implementing something you are not an expert with.

No worries, at the very least you got basic MQTT support working, which is a lot better than nothing!

I definitely agree with bringing the implementation in line with other projects and easing the integration into HA and similar services, so I am very grateful for your help! Your choice of topics seems to make sense for me, this is a lot more transparent and easy to use than the current system. Maybe we can add speed, intensity and palette, but that could also be done at a later stage.

Yes, I was thinking about adding topics for speed, intensity and palette too, but I wasn’t sure how useful they would be. The Home Assistant MQTT Light component doesn’t have corresponding controls for those options (other home automation platforms appear to be similar) so they would only be useful in HA if:

  1. You’re using a custom version of the MQTT Light component that you’ve added these controls to.
  2. You add input_* control components (like input_select, input_boolean or input_number) and use automation rules or an AppDaemon script to map them to the relevant topics for each light.
  3. You could also map speed and intensity to the whitevalue and colortemperature controls, since we don’t use them. (You would still have to add another component (like input_select) and write an automation rule to handle the palette for each light.)

However, in all of these cases you can do the same thing by just using the API topic. You’re already implementing custom scripts or automation rules, at which point it’s trivial to just decode and encode for the API topic.

That said, it’s trivial to add them, so if it will make it easier for someone to integrate into their platform or custom code I can add them to the list!

In fact I haven't been in contact with anyone who used MQTT outside of the home automation use case. HA is by far the most common, but there are users of openHAB and domoticz. Of course I will notify them about the breaking change, there is a high chance that your proposal will benefit the complexity of their implementation as well though!

Thanks!

About segments, the ESP basics are already implemented, just the Javascript UI and a few finishing touches are missing. I like your proposal, structuring the topics like that is very logical! You can take a look at wled19_json.ino to get an idea how to access them. Basically, you can use the strip.getSegment(index) method to access the Segment data object, which contains all the important variables of each segment: … strip.getMaxSegments() will be helpful, too. If you want to support setting segment bounds via MQTT (i'm not sure whether that would make sense), use strip.setSegment(index, start, stop). Let me know if there is anything you need, I'm open to changes to the internal Segment api!

Okay, cool. I’ll start diving into this over the weekend and get a first pass implemented. I may be in contact with some questions as I go!

A consideration to make with segment support in general is which segment's values to return via the default APIs (XML and MQTT without segment number in topic). I don't think that returning the first segment would make sense for all users, so I'm planning to make this "return segment" user configurable. It is hardcoded to 0 right now, but you can already use strip.getReturnedSegmentId()

Yes, I was thinking about this earlier. I think the easiest way to handle this on the MQTT side would be to always include the segment number in the topic path. So, even if the user doesn’t use segments their topic would be Device Topic/0/Command because 0 is the default segment consisting of the entire string of LEDs.

I have two small questions:

  • What about the group topics (like /wled/all)? HA probably won't use them (because I assume there are better ways to group devices). Do you think there are use cases for them or should we drop them completely? If we keep them, they should of course mirror the device topic names, but only subscribe to the /set topics, not publish anything.

See my earlier reply for more on this.

  • I'm a little worried that subscribing and publishing to so many topics (especially if we want to support segments) might cause performance issues or even crashing (because of memory overflow by simultaneous requests or similar). We need to make sure that doesn't happen, but for QoS 0 and local MQTT brokers, it could work out.

I don’t think this will be a big issue, but I planned to do some performance profiling and see how far I can push it. The only place we may run into an issue is if the user has a lot of segments. In those cases they might have to use an ESP32.

The alternative is to only have a single topic per segment which would be a JSON input and output. Home Assistant’s MQTT Light component supports three types of schema: default, JSON and template. Right now I’m designing it for the default schema, where the various controls have their own topics. We could switch to the JSON schema, which basically expects a status topic that contains a JSON dictionary, likewise it sends a JSON dictionary to the command topic. Here’s an example:

{
  "brightness": 255,
  "color_temp": 155,
  "color": {
    "r": 255,
    "g": 180,
    "b": 200,
    "x": 0.406,
    "y": 0.301,
    "h": 344.0,
    "s": 29.412
  },
  "effect": "colorloop",
  "state": "ON",
  "transition": 2,
  "white_value": 150
}

So each segment would subscribe to one topic and publish to one topic: Device Topic/Segment => Status JSON Dictionary Device Topic/Segment/set <= Command JSON Dictionary

However, there's a trade off... These JSON payloads can be hundreds of bytes in size, so we’d need large buffers to hold the incoming data before parsing. Additionally, we’d have to up some of the TCP settings to get it to work reliably.

(You have no idea the hoops I had to jump through to get IR-Blaster to fully support MQTT. IR codes can often be 500+ characters long, that’s not even counting the overhead of putting them in a JSON container!)

It’s doable, but it’ll be a little more complex to implement than the default schema. It really all comes down to where our bottleneck is as to which will perform better.

Thanks once again! Looking forward to hearing from you and let me know if there are any issues!

And thank you for this great project! I’m happy to be able to contribute something.

timothybrown commented 5 years ago

I argree its not too hard for me to change. I new around here and use openhab mqtt for control.

Looking at the HA docs for mqtt the JSON schema alows the most features to be used. https://www.home-assistant.io/components/light.mqtt/#json-schema

Yes, compared to the default schema, the JSON schema gives you a flashing/strobing control and a control to set the transition time between colors. The former isn’t really applicable to WLED and I’m not sure how useful the latter would be.

If you guys would prefer the JSON schema (as described at the end my last post) I could absolutely implement it instead. Technically it would be the most flexible, it’ll just be a lot more complex to implement.

aidbish commented 5 years ago

i think the using the json schema would give greater flexibility going forward.

I like the idea of the speed and intensity of an effect being exposed via MQTT as this would for me enable me to set effects with certain speeds in nodered be able to use a zigbee remote to use

brentbrooks70 commented 5 years ago

@timothybrown

I don’t think this will be a big issue, but I planned to do some performance profiling and see how far I can push it. The only place we may run into an issue is if the user has a lot of segments. In those cases they might have to use an ESP32.

When you are ready to performance test let me know, I can script some pretty rapid fire calls via QIOT to see how fast it can accept the calls and just how quickly it chews up memory for both platforms.

Very excited about these changes, WLED just keeps getting better!!!

timothybrown commented 5 years ago

i think the using the json schema would give greater flexibility going forward.

Yes, I tend to agree, however...

I like the idea of the speed and intensity of an effect being exposed via MQTT as this would for me enable me to set effects with certain speeds in nodered be able to use a zigbee remote to use

Keep in mind that you’d already be able to do that by just passing the correct command (for speed, intensity or palette) to the API topic. The real question is if it’s worth it to expose those commands as their own unique topics. It seems like in your case either would work, though exposing them as topics would make it simpler for you to implement.

What I’m kind of thinking about right now is maybe going with the topics I outlined in my original post, but changing the API topic to accept a JSON input and provide a JSON output. The JSON schema wouldn’t necessarily conform to the default HA JSON schema, in order to save memory, but would still expose all the available commands. This would be fine as HA would be using the default schema for all the basic functions, the JSON topic would just be there for advanced users or people like you using other platforms.

Hopefully that makes sense, I’m just thinking out loud here.

When you are ready to performance test let me know, I can script some pretty rapid fire calls via QIOT to see how fast it can accept the calls and just how quickly it chews up memory for both platforms.

Very excited about these changes, WLED just keeps getting better!!!

Awesome, this will be very helpful. I’ve only got two custom lamps that have 70 LEDs in them (that I mainly use with the fire effect), plus an additional strip of 100 on a wall as bias lighting (I’m slowly expanding that one to eventually encircle the entire room). So testing with larger arrays or even larger numbers of smaller arrays will be great. (The latter is important because network traffic can seriously slow things down, especially when you’ve got a bunch of WiFi devices all rapidly sending packets).

Aircoookie commented 5 years ago

@brentbrooks70 See #208 !

@timothybrown

Yes, I kind of feel that MQTT grouping could be dropped without affecting anyone.

Seems like we reached a consensus. Let's drop it! If the user only wants to connect some of the lights to the broker for some reason, they can still use UDP sync!

You could also map speed and intensity to the _whitevalue...

That is a possibility, although the custom implementation seems more straighforward. Having white value and color temperature do something completely different would confuse new users and it still doesn't cover palettes. That said, WLED does offer RGBW support for WS2813 strips. The bool useRGBW is true if the user enabled it. The white value is either col[3] or for segments, the 8 most significant bytes of the color uint32_t. Maybe it is possible to add that topic when RGBW is enabled, not sure on that one!

So, even if the user doesn’t use segments their topic would be Device Topic/0/Command because 0 is the default segment consisting of the entire string of LEDs.

Agreed! This is a lot easier both for implementation and for the user.

On the topic of JSON vs. topic-per-property scheme types: I think you should implement this however you see fit! If there are any issues down the line we can still switch to the other type.

My thought is that the JSON API is new and maybe his plan is to eventually switch over to it.

This exactly! MQTT was implemented before the JSON API and I have not yet connected them. Since JSON has become easier to deal with in just about any modern environment, I think this reorganization is a good opportunity to provide a JSON state API topic for MQTT in addition to or instead of the XML one. This shouldn't be too much work because we can just hook to the provided functions serializeState(JsonObject) and deserializeState(JsonObject). Heres the API doc

Okay, cool. I’ll start diving into this over the weekend and get a first pass implemented. I may be in contact with some questions as I go!

Awesome, thanks!

timothybrown commented 5 years ago

What do you guys think about adding some general status topics to the list? These would be published under the root device topic only and not duplicated for each segment. Here's an example of what Espurna publishes:

Bathroom-Vanity-Left/app ESPURNA
Bathroom-Vanity-Left/version 1.13.5
Bathroom-Vanity-Left/board AITHINKER_AI_LIGHT
Bathroom-Vanity-Left/host Bathroom-Vanity-Left
Bathroom-Vanity-Left/ssid ********
Bathroom-Vanity-Left/ip 10.***.***.***
Bathroom-Vanity-Left/Mac **:**:**:**:**:**
Bathroom-Vanity-Left/rssi -52
Bathroom-Vanity-Left/uptime 233119
Bathroom-Vanity-Left/datetime 2019-08-24 21:15:12
Bathroom-Vanity-Left/freeheap 21944
Bathroom-Vanity-Left/vcc 3246
Bathroom-Vanity-Left/status 1
Bathroom-Vanity-Left/loadavg 1

Adding these wouldn't really take up additional resources as we wouldn't be subscribing to them and they could be updated every 60 seconds or during otherwise idle time. While HA and other platforms won't directly use them, I personally find them useful, especially for development or new installations. (I run a custom Python script that tracks this information, which I can review to make sure there's no longterm memory leaks, WiFi issues, stuff like that.)

So, this is what our list looks like now:

aidbish commented 5 years ago

Yep i like, you could make sensors in HA for things like, fw version, ip address which saves having to load the gui and could be on a glance card or more info card, someone may even make a custom card for WLED that exposes all options.

timothybrown commented 5 years ago

@aidbish Yup! I was just reading through some HA documents and came across another way that might be viable as well. Instead of having all those individual status topics, we could just have a single one that publishes a JSON payload, like so:

Device Topic/attributes =>

{
  “Firmware”: “WLED xx.yy.zz”,
  “VCC”: “3300”,
  “RSSI”: “-50”,
  “MAC”: “00:DE:AD:BE:EF:00”,
  “Hostname”: “wled.local”,
  “IP”: “127.0.0.1”,
  “SSID”: “Generic Router”,
  “LoadAvg”: “1.0”,
  “FreeMem”: “50000”,
  “Uptime”: “300”
}

Now, the advantage here is that HA will actually display this information in the frontend without requiring you to add additional sensors (the attributes will be at the very bottom of the card). You can also access the data from scripts, automations or even the UI via simple templating! (An example would be {{ state_attr('light.wled', 'IP') }} for the IP.)

To get this working, all we need to do is include an entry for json_attributes_topic that points to Device Topic/attributes in our HA Discovery config payload.

This seems like an overall much cleaner and more useful way to do it. What do you guys think?

aidbish commented 5 years ago

That would work, i am happy with either way, as long as we get the correct status of the strip and can manipulate its options.

i think once done, we post on the HA about this, At the moment i am finishing off some outdoor glass rock lights with 16pixel circular strips, (using recent build of WLED) so happy to do some show and tell when mqtt is working on above to promote WLED.

timothybrown commented 5 years ago

Okay, here’s my revised list. Did I forget anything? Offhand I can’t think of a scenario that this wouldn’t cover.

Segment Topics

Non-Segment Topics

aidbish commented 5 years ago

Looks good from me. A good outcome.

brentbrooks70 commented 5 years ago

@timothybrown That looks very good, the JSON attribute response makes way more sense than the individual topics to me.

timothybrown commented 5 years ago

Okay, cool! So that’s what I’m going with then. I should have a test branch available sometime this week. This has taken a bit more time than I initially anticipated, mainly because I had to fully get back into the mindset of programming in C/CPP. (I didn’t realize just how much I’d started taking for granted simple Python functionality like string and type handling!)

brentbrooks70 commented 5 years ago

@timothybrown LOL, Strings??? No such thing in C! Let me know when you have something I can stress test. I've got both ESP platforms set with 144LEDs (144/m) as well as both platforms with 300 LEDs (60/m), in a test environment, and an 8266 with 700 LEDs (60/m) ready for testing (this last set is in my daughters room so I can only test during weekdays while she is at school). I have also designed a Node-Red flow that will generate up to 100 messages per sec. Will start with something much slower initially but would like to push it until I kill it, Should I have the board reply to each message with a freeheap (not sure how that would impact memory) or maybe poll the HTTP freeheap at certain intervals to get an idea of memory consumption? Not sure which method would cause less of an impact on speed or memory use....thoughts? Will there be a reply to every command/message? If so, would it contain freeheap?

Aircoookie commented 5 years ago

@timothybrown Awesome! While looking at the topic list, I forgot something though. Although by far not used by all effects, there is a secondary (background) and even tertiary color. We can either add topics for those or just use the JSON API. Totally agree, when Python is like building a computer, Cpp is like designing the CPU!

@brentbrooks70 Awesome setup! I bet your daughter loves her room! If you want to stress test for X amount of LEDs by the way, you don't need to have them physically connected. Just selecting the amount in LED settings is enough. I've just pushed an update to master that fixes a critical issue with the JSON api (HTTP POST requests had no effect). I plan to release it as 0.8.5 soon. If you'd like to, stresstest it! About freeheap, the new /info topic will provide it via MQTT. Or just poll the HTTP as you said. I've found the ESPs to be quite resilient to about 50 requests per second. Above that they are handled too slowly and heap crash will occur. Also another developer has found an apparent issue in the TCP library that can cause memory leaks if there are multiple IPs sending requests to the ESP simultaneously (https://github.com/me-no-dev/ESPAsyncWebServer/issues/528). Would be interesting to know how much of an issue this is in practice!

timothybrown commented 5 years ago

@Aircoookie

@timothybrown Awesome! While looking at the topic list, I forgot something though. Although by far not used by all effects, there is a secondary (background) and even tertiary color. We can either add topics for those or just use the JSON API. Totally agree, when Python is like building a computer, Cpp is like designing the CPU!

Huh, yeah, that's a good point about the secondary/tertiary colors... The first thing that jumps to my mind is, what if we allow inputting that via the /color topic? We could optionally accept a JSON list like so: {[[255,255,255], [0,0,0], [127,127,127]]} (Primary, Secondary, Tertiary)

The color status topic would report back in the same way, but only if the command is given in this JSON format.

Normally, I'm not keen on having a command channel behave in more than one way (it tends to greatly complicate end user implementation), but this one might not be too bad. I'll need to think on it a bit more.

@brentbrooks70

@timothybrown LOL, Strings??? No such thing in C!

Ugh, I know right? When I code, I like to go through and lay out the general structure of the program in comments first, then I go back and actually write the code under the comments. It's sort of like using pseudocode with the side benefit of basically getting free documentation along the way! Anyway, I'm so used to just being like, "Here we concatenate the device topic and command together," thinking hey, that's easy, just a loop and the "+" operator, then I remember this is C and I'm going to have to create a char array for a buffer, call some "string" handling functions, be mindful of memory constraints and so on. But, once you get in the right mindset it's not so bad. Just different, and a bit more typing!

That sounds like an awesome setup you’ve got going there!

You'll be able to get free heap out of the /info topic. I've called it FreeMem in my example above, but that should read FreeHeap instead.

brentbrooks70 commented 5 years ago

If you want to stress test for X amount of LEDs by the way, you don't need to have them physically connected. Just selecting the amount in LED settings is enough

@Aircoookie Without having the LEDs attached how would you visibly know if you are impacting the update speed of each "frame" or causing "hickups"?

I bet your daughter loves her room!

@Aircoookie Even more so thanks to you and WLED!!

Normally, I'm not keen on having a command channel behave in more than one way (it tends to greatly complicate end user implementation), but this one might not be too bad. I'll need to think on it a bit more.

@timothybrown I agree, multiple behaviors on the same command could become confusing; if required heavy documentation might be the only saving grace.

timothybrown commented 5 years ago

@Aircoookie I know this is off topic, but are there any plans on moving configuration options over from "EEPROM" storage to SPIFFs? We're starting to run up against constraints and the implementation is becoming exponentially more complex as we add to it. Moving configuration options over to an INI file stored in SPIFFs would greatly simplify things I think.

The reason I bring this up is because I'd like to add another option on the MQTT page to enter the Home Assistant discovery base topic. (If the user leaves it blank we'll disable HA Discovery.)

timothybrown commented 5 years ago

Here's the configuration JSON we'll be sending to Home Assistant.

{
  "~":"Device Topic/Segment",
  "name": "WLED Light (Segment)", # Settings->User Interface->Server Description + "(" + Segment Number + ")"
  "platform":"mqtt",
  "state_topic":"~/switch",
  "command_topic":"~/switch/set",
  "payload_on":"on",
  "payload_off":"off",
  "availability_topic":"Device Topic/status",
  "payload_available":"online",
  "payload_not_available":"offline",
  "brightness_state_topic":"~/brightness",
  "brightness_command_topic":"~/brightness/set",
  "rgb_state_topic":"~/color",
  "rgb_command_topic":"~/color/set",
  "white_value_state_topic":"~/white",
  "white_value_command_topic":"~/white/set",
  "effect_state_topic":"~/fx",
  "effect_command_topic":"~/fx/set",
  "effect_list":["Solid", "Blink", ... "Railway", "Ripple"],
  "json_attributes_topic":"Device Topic/info",
  "uniq_id":"WLED-BEEF00_0", # "WLED-" + Last Six of MAC + "_" + Segment Number
  "device":{
    "identifiers":["WLED-BEEF00"], # "WLED-" + Last Six of MAC
    "name":"WLED Light", # Settings->User Interface->Server Description
    "manufacturer":"WLED",
    "model":"Board Name",
    "sw_version":"WLED xx.yy.zz"
    }
  }

Obviously I'll be using the abbreviated versions of the dictionary keys to save on memory. I've included the full names here for ease of reading. The tilde entry allows us to shorten the topic names. HA will automatically replace future instances of tilde with the full Device Topic/Segment name.

timothybrown commented 5 years ago

Implementation Detail/Note to Future Self: Add a hook to the segment API so that when a user deletes a segment we call a function that deletes it from HA as well (by sending an empty configuration payload to the Home Assistant discovery topic).

(Sorry for the comment spam guys, this just helps me to stay better organized.)

aidbish commented 5 years ago

no need to apologise. as a non developer i like trying to understand the process.

JAMESBOWLER commented 5 years ago

Timothy you need to understand somthing that took me years to comprehend. Every one of you contributions and comments on here have helped someone understand what you are doing and what you were thinking at the time. Without these insites, questions and examples the project may lead off in a diferent direction.

Aircoookie commented 5 years ago

I know this is off topic, but are there any plans on moving configuration options over from "EEPROM" storage to SPIFFs? We're starting to run up against constraints and the implementation is becoming exponentially more complex as we add to it. Moving configuration options over to an INI file stored in SPIFFs would greatly simplify things I think. The reason I bring this up is because I'd like to add another option on the MQTT page to enter the Home Assistant discovery base topic. (If the user leaves it blank we'll disable HA Discovery.)

Just opened #211 for this! For now I'd propose you use the MQTT group topic bytes as we'll no longer use them. I totally agree with you, the current implementation with the arbitrary integer memory locations and having all the EEPVER exceptions is lacking.

Also I agree with the others, not writing down one's thoughts or alternatives is kind of like writing code without comments. So I highly appreciate that you do!

timothybrown commented 5 years ago

@Aircoookie

Just opened #211 for this! For now I'd propose you use the MQTT group topic bytes as we'll no longer use them. I totally agree with you, the current implementation with the arbitrary integer memory locations and having all the EEPVER exceptions is lacking.

Ah, oh course, we will have some free space since we’re removing the group topic! I had a reply typed up to your original post, but I’ll post it in #211.

@Aircoookie

Also I agree with the others, not writing down one's thoughts or alternatives is kind of like writing code without comments. So I highly appreciate that you do!

@JAMESBOWLER

Timothy you need to understand somthing that took me years to comprehend. Every one of you contributions and comments on here have helped someone understand what you are doing and what you were thinking at the time. Without these insites, questions and examples the project may lead off in a diferent direction.

Yes, that’s very true!

For most projects I like to open an issue for everything I work on and use it as a bit of a development log, because a year from now when someone else wants to add onto my code, I feel like they’d appreciate knowing my thought process when I was writing it (I know I appreciate it). Comments are great for explaining how individual pieces of the code work, but I’ve found more often than not if I can understand what the programmer was thinking when he wrote it, the “why” and maybe some of the “who”, it makes things a lot clearer. It also helps me to figure out what I was thinking when I go to fix a bug six months down the road! (There have been times when I’ve started reading through source code and think to myself, “Man, what idiot wrote this garbage?”, then I scroll up to the header and realize whoops, I wrote it, 5 weeks/months/years ago.)

I’ve worked on projects in the past that were really strict about “misusing” the bug/issue tracker, so I’m always a little cautious when starting out in a project. I’m glad you guys don’t mind!

Another benefit is it allows other developers to validate or find flaws in my logic/code/concept as I go, which is immensely helpful. (On top of that you get input from end-users.)

prankousky commented 5 years ago

I have tried using WLED with Home Assistant, but was not able to get satisfying results, so this thread really got my hopes up.

Are any of you successfully using the current version of WLED with Home Assistant? If so, would you please share your configuration? The template from the Wiki is great, but I am missing options to change palettes.

timothybrown commented 5 years ago

@prankousky The changes being discussed in this thread have not yet been merged into WLED. In the current version the very basics work (setting power, brightness and FX), but the status reporting from WLED to HA is broken. I’m sure you could come up with a passable config using the XML API topic and templating, but it would ultimately be a waste of time, as it will break when my update gets merged in. I’m hoping to have a beta version of the new MQTT/HA code up on my fork of the repo this weekend, so just hang in there a bit longer, it’ll be worth the wait!

As for palettes, unfortunately Home Assistant does not provide support for palettes in the MQTT light component. That component only provides controls for power, color, brightness, white level, color temperature and effects. That said, I am exposing the palette (and other) options as topics, so it would be possible to implement palette support via something like the input_list component and either a set of automation rules, a script or AppDaemon.

As an aside I should mention that once I get the new MQTT/HA code stable and released I will start looking into making a custom version of the MQTT Light component that exposes these additional options (palettes, presets, secondary/tertiary colors) that people can install if they want. I don’t think it will be that hard, but it’ll most likely be a couple of months before you see it.

timothybrown commented 5 years ago

Okay, so I’ve again updated the list. I decided to give the primary/secondary/tertiary colors their own topics under the color hierarchy. Personally, I feel a better way to handle this would be by adding an option to the API to let the user setup custom palettes and then just select said palette. (However, that’s a discussion for a later time.)

This is tentative. If we have performance problems the new secondary/tertiary topics will be the first to be dropped.

Segment Topics

Non-Segment Topics

Aircoookie commented 5 years ago

@timothybrown Awesome, having separate topic for them might not be a bad idea! I would however propose renaming them to DeviceTopic/SegmentId/color/0 and so on, using numerals. This would allow us to extend the number of colors at some point (which I won't do since for that palettes are the correct tool, the only reason I haven't switched to palette-based color handling exclusively is the white-channel support). But this syntax would allow for supporting custom palettes over the color topic as well. Custom palettes are something I am definitely planning, so it's great to hear you've been thinking about them as well! But I'm not yet sure on merging the concepts of colors and palette, maybe for custom palettes we'd rather use a JSON array of the colors on a separate topic. Let's keep that discussion for another time though!

Another point I wanted to bring up is that I noticed some users solely use the MQTT implementation to send HTTP API requests aka something like T=2. It could be benefitial for the /api/set topic to still accept that syntax alongside JSON (by falling back to it if JSON parsing fails, or just checking for the { character). You could use this code from the current implementation to call the HTTP API handler:

String apiReq = "win&";
apiReq += (char*)payload;
handleSet(nullptr, apiReq);

Thanks once again for your hard work! I really like the new topic structure!

sujitrp commented 5 years ago

With current 0.8.3 version MQTT integrated the WLED with OpenHab

image

https://github.com/Aircoookie/WLED/issues/35#issuecomment-530479211

timothybrown commented 5 years ago

Hey guys, sorry for the delay! Food poising sucks.

I’m back on this today and will be submitting a pull request soon!

aidbish commented 5 years ago

Mate, no need to be sorry. Hope your feeling better though? food poisoning isn't fun

flashy02 commented 5 years ago

@timothybrown : I have found this isue and i am kind happy to read about the planed MQTT refactoring. Is there a timeline when it will be available? At the moment I'm playing with wled an I would wait for integration in OpenHAB until the new MQTT interface is available.

EvasionOfTruth commented 5 years ago

I would love for this to be implemented.

bkpsu commented 5 years ago

@timothybrown - great work with this so far, excited to see it implemented in WLED, as I'm a new convert/user and LOVE the work @Aircoookie has done with this project (all the different effects, interfaces, just awesome stuff!). I've already got WLED installed on one of my Kube LED controllers, and was getting ready to bring it into my openHAB setup, when I learned about your rewrite of the MQTT interface,

I do have one "dissenting" comment about removing the group topic. While it is true that most of the home automation platforms have a way to group devices, without a group topic, the OH platform needs to send individual messages to each device in the group. With an increasing # of devices (I have a LED strip in every window of my house, configured to synchronize effects for the holidays), the message stack causes delays, and results in a "cascading" fx change. With a group topic, all devices instantly synchronize to the one message meant for all of them. That is the benefit of a device-side group topic, IMO.

Of course, there is a workaround, to have all devices subscribe to the same \set topic, but then you lose ability to control each device individually.

Just my $0.02/suggestion. Keep up the great work!

bagobones commented 5 years ago

Hi All, I just discovered WLED, I have been banging away on some custom code based on https://github.com/thehookup/Holiday_LEDs_2.0/blob/master/Holiday_Lights_2.0.yaml which has segment support.

There might be some value in having WLED present one topic but still have segments.. IN the hookup code effects are played in all zones with the same settings but having zones changes the way they appear, IE lights bounce IN each zone or fire appears to burn from the ends of zones.. It is easy to control but looks more custom to your house or installation.

Other than that I would still be super happy with segments each appearing as a discovered light and not bothering with grouping.

I think the main value to having group topics is syncing things and keeping traffic down, if that is the case a group topic for ALL WLEDs (if you are running several instances) and ALL segments in a multi segment unit would be useful for some. Not completely necessary however.

TheFuzz4 commented 5 years ago

@bagobones that is exactly what I'm looking to do once I get my strips in. I'd also like to be able to control multiple zones from the various GPIO pins of the NodeMCU. Thank you @Aircoookie and @timothybrown for your continued hard work. Looking forward to installing these this year.

Miesjelangelo commented 5 years ago

Is there an update on when this will be released in an update? I have WLED installed but not yet connected to Home Assistant because of this. Should I wait or do you advice to just implement it and change it when the update arrives (since this will be a breaking change if I update the WLED controller)?

Good work so far, looks promising!

Aircoookie commented 5 years ago

I believe @timothybrown commented recently that he wasn't feeling too well. If he doesn't respond soon, I will assume that he needs a break or more time for other projects and I will try implementing his proposed structure myself :)

jhuizingh commented 5 years ago

I'm entering the conversation a little late in the game, so if this change is already in motion I think it's a step in the right direction!

It does seem kind of specific to HA. I use openHAB, and something that would make MQTT as easy as possible to implement is for each different item that can be set to have a unique topic. The reason for this is that in openHAB you define and item, then you can set inbound and outbound mqtt topics in the item metadata. If an item does not have its own topic, then it would have to be parsed from a message that has the info for multiple values. For example:

Switch LEDStrip_Power "LED Lights" (gLEDStrip_WLED_Power,gOutdoorLights) ["Lighting"] 
    {mqtt=">[mosquitto:wled/test:command:*:default],
    <[mosquitto:wled/test/v:state:XPATH(concat(substring('OFF',1,number(not(not(//ac/text()[.='0'])))*3),substring('ON',1,number(not(//ac/text()[.='0']))*2)))]"}

For example, the Power can output directly to the device topic since it is unique for that purpose. However, from my limited experimenting it looks like that info is only possible in the tag on the /v incoming topic. There are a number of different ways to transform input, but they all have some pitfalls. So I had a few options:

  1. Create an XPATH transform. That's what I did above. It's convoluted at best but converts 0 to OFF and any nonzero number to ON. The pro is that it's all inline.
  2. Run the /v data through a Javascript transformation function or maybe an XSLT transformation. Ideally it would be reusable, but as-is at the point of the transformation function we don't know what item/topic the xml is for, so I'd have to create a different transformation file for each discrete piece of WLED data that I wanted in my system ( wled_ac.js, wled_cl.js wled_cs.js, wled_fx.js etc..)

Both of those are tricky or don't scale well to many items.

Sending would be similarly interesting. Probably the best way would be to create a rule for when the majority of the items change, and then pass the proper api format string to /api based on which item has changed (i.e. LEDStrip_Effect -> send /api: FX=value). Not ideal because there would have to be a mapping of item name to parameter string that needs to be sent to the topic. My ideal mqtt topic format would probably match with the HTTP Request Api and be something like this:

[mqttDeviceTopic]/A [mqttDeviceTopic]/A/set [mqttDeviceTopic]/T [mqttDeviceTopic]/T/set [mqttDeviceTopic]/FX [mqttDeviceTopic]/FX/set [mqttDeviceTopic]/SX [mqttDeviceTopic]/SX/set etc...

It could also use any string such as the ones used in the JSON api instead for the property names. I'm not too concerned which ones are used. I figure code-wise it may be easiest to use what's already defined for one of the other apis.

With the above proposed MQTT topics, you could subscribe to

[mqttDeviceTopic]/# which would match all of the above. Then it could be parsed out using regular expressions.

The regular expression ^[mqttDeviceTopic]\/(\w+)(?:\/(\w+))?$ would match any of the above examples. $1=FX . $2=set . when the topic that received the publish is [mqttDeviceTopic]/FX/set. See examples here: regexr.com/4noei

After a match, you could use the value of $1 to figure out which property to update, similar to how the HTTP Binding does it (I assume).

^[mqttDeviceTopic]\/(\d+)\/(\w+)(?:\/(\w+))?$ See examples here: regexr.com/4nof1 If this regex was run first, it would catch any segment items because of the \d+ argument, then the previous regex would catch all "top level" topics.

Implementing this would allow openHAB items to be defined more like this:

Switch LEDStrip_Power "LED Lights" (gLEDStrip_WLED_Power,gOutdoorLights) ["Lighting"] 
    {mqtt=">[mosquitto:wled/FX/set:command:*:default],
    <[mosquitto:wled/FX:state:JS(FXStringNumberMap)]"}

...Much simpler and more reusable.

Does this look worthwhile at all?

I'm pretty rusty in my C code, otherwise I would try implementing this myself.

Aircoookie commented 5 years ago

@jhuizingh Yes, I believe your proposal is pretty much what we are already going for! We need to make sure that the new topics are useful for as many users as possible!

@timothybrown the only difference I see to your proposed structure at first glance is the absense of the segment ID. I don't believe this is an issue since any application that does not want to support segments can just use [mqttDeviceTopic]/0 as root topic.

jhuizingh commented 5 years ago

@Aircoookie my schema actually attempted to support @timothybrown's as much as possible. It can support the segment id stuff too. The second regex above: 

^[mqttDeviceTopic]\/(\d+)\/(\w+)(?:\/(\w+))?$

Handles the segment id through the (\d+) portion as shown at regexr.com/4nof1

bagobones commented 5 years ago

While I use HA, I think making the topics as universally compatible is a good idea. Since HA supports auto discovery how the topics work on HA wont matter as much to average HA users anyway.