Add SSL support for MQTT

[copyrights]

• Replace PubSubClient with AsyncMqttClient library
• Add new setting MQTT secure
• Add new setting MQTT server fingerprint

[sidoh]

This is excellent. How much testing have you done?

[copyrights]

My use-case is milight remote -> esp8266_milight_hub (NRF24L01) -> MQTT (user/pw,SSL+Fingerprint) -> mosquitto -> OpenHAB. So I mainly focus on that with "MQTT update topic pattern".

I did some basic {"state":"ON"} {"state":"OFF"} tests with "MQTT topic pattern".

When I active "MQTT state topic pattern" the MQTT message only shows a single "{". As I didn't use that feature (nor esp8266_milight_hub) before, I don't know what to expect here.

Furthermore I did some negative testing with a wrong fingerprint. The esp8266 didn't connect to the broker (as expected).

[copyrights]

One more comment on my implementation.
As the library name indicates, MQTT now runs asynchronous. So there is no need for a .loop() in that library. Which would be great, but it also means that the callback are running outside the setup/loop context. And that causes that yield() is no working.
So unfortunately I'd to implement a loop-like poll mechanism :-(

[sidoh]

The state topic message should always be valid JSON. Perhaps published messages are getting clipped somewhere?

re: asysnc, makes sense. This is the main reason I asked the question, actually. I'd love to refactor more of the library to be async-compatible because this is the main hurdle to using an ESP32. This has felt like a pretty significant undertaking to me, but perhaps I'm overestimating.

[copyrights]

Oh, I think I just found the issue with the MQTT state behaviour. I will fix and test it.

[copyrights]

Found it, fixed it, tested it. MQTT state now publish state as valid JSON.

[sidoh]

Nice, good find. Looks like that fixed the clipped message problem.

I think we might need to poke at the asynchronicity a bit more. We're losing packets.

For example, if I spam 8 command messages really quickly like so:

$ for i in $(seq 1 8); do mosquitto_pub -h mqtt -p 1883 -u me -P hunter2 -t 'milight2/commands/0x1/rgb_cct/'$i -m '{"status":"on"}';  done

This is what I see on MQTT:

$ mosquitto_sub -h mqtt -p 1883 -u me -P hunter2 -t 'milight2/+/+/+/+' -v
milight2/commands/0x1/rgb_cct/1 {"status":"on"}
milight2/commands/0x1/rgb_cct/2 {"status":"on"}
milight2/commands/0x1/rgb_cct/3 {"status":"on"}
milight2/commands/0x1/rgb_cct/4 {"status":"on"}
milight2/commands/0x1/rgb_cct/5 {"status":"on"}
milight2/commands/0x1/rgb_cct/6 {"status":"on"}
milight2/commands/0x1/rgb_cct/7 {"status":"on"}
milight2/commands/0x1/rgb_cct/8 {"status":"on"}
milight2/updates/0x1/rgb_cct/1 {"state":"ON"}

Notice that there's only one update. It seems like the ESP is dropping packets.

If I spam the command a couple of times, I see one or two more packets sent, but it's never the full 8.

For reference: the same test on 1.7.1 passes:

milight2/commands/0x1/fut089/1 {"status":"on"}
milight2/commands/0x1/fut089/2 {"status":"on"}
milight2/commands/0x1/fut089/3 {"status":"on"}
milight2/commands/0x1/fut089/4 {"status":"on"}
milight2/commands/0x1/fut089/5 {"status":"on"}
milight2/commands/0x1/fut089/6 {"status":"on"}
milight2/commands/0x1/fut089/7 {"status":"on"}
milight2/commands/0x1/fut089/8 {"status":"on"}
milight2/updates/0x1/fut089/1 {"state":"ON"}
milight2/states/0x1/fut089/1 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/updates/0x1/fut089/2 {"state":"ON"}
milight2/updates/0x1/fut089/3 {"state":"ON"}
milight2/updates/0x1/fut089/4 {"state":"ON"}
milight2/states/0x1/fut089/2 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/updates/0x1/fut089/5 {"state":"ON"}
milight2/updates/0x1/fut089/6 {"state":"ON"}
milight2/updates/0x1/fut089/7 {"state":"ON"}
milight2/states/0x1/fut089/3 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/updates/0x1/fut089/8 {"state":"ON"}
milight2/states/0x1/fut089/4 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/states/0x1/fut089/5 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/states/0x1/fut089/6 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/states/0x1/fut089/7 {"state":"ON","color":{"r":255,"g":255,"b":255}}
milight2/states/0x1/fut089/8 {"state":"ON","color":{"r":255,"g":255,"b":255}}

I'm guessing this is because your callback only buffers one message. We should probably buffer more than that. I would suggest using CircularBuffer, which is already included. Check out BulbStateUpdater for an example.

One last point -- free heap is about 5 KB lower on this branch (that's about 20% of free heap on the 1.7.1 branch). Guessing that's just TLS, but I haven't confirmed yet. If there's any low-hanging fruit you're aware of to trim memory usage, we should probably explore that.

[copyrights]

About the free heap it seems to be SSL. If you compile without ASYNC_TCP_SSL_ENABLED in build_flags, free heap does increase. I have a SSL-only broker, so I can't test how much it is after a connection is established.

Your test results are as I would expect.

if(!mqttmsg) // For now irgnore new message while process last message.

I will look into CircularBuffer.

[copyrights]

I did some quick testing with CircularBuffer. I did what I (may) would do if there where tons of memory available. I pushed topic and payload strings to it. As expected this was not very feasible.

Next I wanted to know how PubSubClient performs. So I setup a plain mosquitto server and uploaded the current master to my board. Than I start flooding ON/OFF mqtt messages to espMH. At 10 repeats I got a nice delayed light show. At 100 repeats I saw the esp working hard, but not a single switching. Obvious I did this to see how high the bar is ;-)

IMHO a good compromise would be to only queue up a certain amount messages/commands and afterwards drop the rest until a threshold level in the queue is reached. Better to drop than to block.

To achieve this I think the messages should be parsed into a memory-efficient command object inside the callback. I haven’t check yet how small this object could get, but wouldn’t except more than a few bytes.
Furthermore it should be checked, if the transmit function needs to be protected from interrupts like the callback function.

Any thought on that?

[sidoh]

Next I wanted to know how PubSubClient performs. So I setup a plain mosquitto server and uploaded the current master to my board. Than I start flooding ON/OFF mqtt messages to espMH. At 10 repeats I got a nice delayed light show. At 100 repeats I saw the esp working hard, but not a single switching. Obvious I did this to see how high the bar is ;-)

The PubSubClient bar is probably set by whatever the SDK's internal TCP buffer can handle given that packets are being handled synchronously.

IMHO a good compromise would be to only queue up a certain amount messages/commands and afterwards drop the rest until a threshold level in the queue is reached. Better to drop than to block.

Yes, this is exactly what I was meaning to suggest with using CircularBuffer. Dropping packets is the correct behavior if the buffer is full.

To achieve this I think the messages should be parsed into a memory-efficient command object inside the callback. I haven’t check yet how small this object could get, but wouldn’t except more than a few bytes.

I haven't thought too much about how to structure the buffer, but yeah, definitely not good to buffer the 400 bytes per message or whatever it is. A couple of suggestions:

1. Use a static char* buffer for all packets. Can use this in the same way a circular buffer works. The actual packet buffer can just store pointers to strings in the static buffer. I expect most messages will be far under the max packet size, and this takes advantage of that.
2. Parse the packet into a GroupState, which is 8 bytes. Store this along with a BulbId which is 4 bytes.

I probably prefer (1) because it's way more straightforward.

[sidoh]

@copyrights no rush, but wanted to check if you'd had a chance to look at this. I'm also happy to help out however I can. :)

[copyrights]

@sidoh sorry, I was quiet busy. It's still at the top on my hobby list. Hopefully I'll find some time for it soon.
I'm actually planing to make it optional, so that it won't bother someone how don't need SSL.

[sidoh]

Oops, think this got closed automatically when I nuked the 1.8 branch. Re-opening.

[Timo2208]

can the pull request be merged?

[copyrights]

@Timo2208 I haven't done anything with this code in almost a year. It works for me then, but I'm not using it my self at the moment. As mentioned in my comment there is still some work to do. Unfortunately I won't do it -.-

[sidoh]

@Timo2208 - no, we'll need to fix the issues discussed earlier in this thread before merging.

@copyrights - no worries, I'll take a stab at it at some point. :)

(also obviously happy to look at updates to the PR from anyone else who wants to have a go at it)

[sidoh]

Packet queuing is implemented in v1.10, which should enable using the async MQTT library.

[poudenes]

Is this still in scope so you can use SSL on MQTT?