Refactor MQTT subscriptions and API calls

[proddy]

Currently we make individual MQTT subscriptions for everything, whether it's a command (e.g. system/send or thermostat/temp) or an endpoint. And there are also options to register all device values which introduces a bit more code, I plan to refactor this and use wildcards and remove the options so all options work.

[proddy]

hi @MichaelDvP, I made some breaking changes to the code so please wait for this PR before submitting any changes. I'll see if I can get it all working tonight.

[proddy]

• renamed add_json() to add()
• removed MQTT subscribed format as we subscribe to device wild cards (See below)
• add MQTT send response topic as a setting
• added command parser to Command() to extract the command and id (if using hc/wwc)
• MQTT subscribes to top-level devices (e.g. boiler/#) instead of individual topics
• Refactored API (WebApiService) to be more versatile, handling responses as json
• Refactored MQTT on_message() to use new API so they have the same parsing logic, saving about 8K of RAM
• increased MQTT buffer from 200 to 300
• removed log messages 'Adding new ...' to where the class is initiated
• 'info_short' command renamed to 'list'
• code cleanup, improved error handling and some minor text re-work
• replaced hacks where possible
• remove wildcards * and #

[MichaelDvP]

There might be an issue with checking the mqtt base, we have introduces the "base" to allow some people to set a longer mqtt-path like tasmota. e.g home/cellar/heating as base. But this will now split by the parser.

[proddy]

Good point, thanks for checking. I touched a lot of code so expect a few more quirks here and there. In this case I can remove that check anyway since the subscriptions will always come in on the MQTT base and API calls on the /api. This was left over from an earlier experiment where I subscribed to '/#' but then it would pick up everything, including the MQTT 'response' topics.

[proddy]

There might be an issue with checking the mqtt base, we have introduces the "base" to allow some people to set a longer mqtt-path like tasmota. e.g home/cellar/heating as base. But this will now split by the parser.

making a fix now

[MichaelDvP]

Maybe it differs for different mqtt brokers, but subscription on thermostat/# does not work for sending to topic: thermostat, payload: {"cmd":"seltemp", "value":21} on iobroker internal mqtt. I have to change subscription to thermostat# or use two subscriotions to thermostat and thermostat/#.

[proddy]

oh you are correct, sending to topic 'thermostat' will not work. I didn't add that. But will later. Good catch!

[proddy]

this change to <device># is breaking Mosquitto, with the errors:

1635963075: Invalid subscription string from 192.168.1.134, disconnecting.
1635963075: Client ems-esp disconnected due to malformed packet.
1635963077: New connection from 192.168.1.134:55498 on port 1883.
1635963077: New client connected from 192.168.1.134:55498 as ems-esp (p2, c1, k60).
1635963079: Invalid subscription string from 192.168.1.134, disconnecting.
1635963079: Client ems-esp disconnected due to malformed packet.
...

It's not allowed according the official MQTT specification, see (example “sport/tennis#” is not valid).

I'll add the / back

--- edit ---

Unlike the API I think we should restrict client's from publishing directly to the device's, such as ems-esp/system or ems-esp/thermostat and enforce the command to be on the path. So topic=ems-esp/thermostat/mode payload="auto" unless there's something weird in ioBroker

[MichaelDvP]

Please add the extra subscription to device only. This was the methode for all emsesp versions until now and first in the documentation. I think most people using this, as individual subscriptions need to be activated by settings.

[proddy]

Ok, I'll add it back if people use it. I still don't see a valid use case of why anyone would publish to an MQTT topic like 'ems-esp/thermostat'. If you have an example let me know!

BTW individual subscriptions and all the complicated settings stuff is gone.

[MichaelDvP]

I've already linked it in doc, it is the default most older users use:
via MQTT in the payload with {"cmd":"<command>" ,"data":<data>, "id":<id>}. The MQTT topic being the <device>
I don't see a reason to make such a breaking change.

[MichaelDvP]

I see in the specification:
“sport/#” also matches the singular “sport”, since # includes the parent level.
the single subscription to <device>/# should work, it's an issue of the ioBroker-server.
I think you can revert keep the spec, i will create an issue for ioBroker.mqtt.

[proddy]

Ok, I'll also test with Mosquitto (which is standard with Home Assistant) and perhaps wrap the additonal subscription code around an #ifdef IOBROKER just in case. I'm out today so if you want to do that pls go ahead

[MichaelDvP]

I dont think this is needed, emsesp should not correct the bugs from other software. Iobroker-users can use @tp1de adapter or mosquitto. I've opend an issue and maybe this will be fixed.

[MichaelDvP]

I've opend an issue, made a small fix and a PR. I'll remove the extra subscriptions, if someone complains, he can use my version until the PR is merged.

[proddy]

ODINServ reported an error via Discord that sending a value in an MQTT payload fails. I've made a fix

[giovanne123]

Can you please help me, I'm a little bit confused.
I updated from v3.2.2b10 to v3.3.0b8.

Now in v3.3.0b8 my previously (in v3.2.2b10) working HA/Mqtt commands are no longer working.

HA:

one_time_water_on:
  sequence:
  - service: mqtt.publish
    data:
      topic: ems-esp/boiler
      payload: '{"cmd":"wwonetime","data":1}'

Results in MQTT Broker:

And in EMS-ESP console:

Manually calling command in console is working:

ems-esp32:# call boiler wwonetime 1
000+01:56:49.210 I 34: [command] Calling boiler command 'wwonetime', value 1, id is default
000+01:56:49.210 I 35: [boiler] Setting ww OneTime loading on

Can you please tell me how the config for the command should look like in HA?
Will this change stay in upcoming versions/releases?
Or should I downgrade to 3.2.x ?

Thanks

[proddy]

yes, sorry, that was a bug I fixed yesterday around 1pm CET. I didn't change the version so it's still called 3.3.0b8. Can you check you have the latest ?

[giovanne123]

thanks, yes my version was older.
Updated again to v3.3.0b8 and now is working like before :-)