ESP32-Daikin icon indicating copy to clipboard operation
ESP32-Daikin copied to clipboard

[BUG] Cannot include `/` in prefixcommand (silent failure)

Open basak opened this issue 3 months ago • 3 comments

Faikin hardware Purchased from Amazon.

Daikin hardware N/A. Software version: Faikin-S3-MINI-N4-R2: 91c1bc5 2024-03-31T10:59:15 S21

Describe the bug

I cannot seem to include / in prefixcommand.

** Steps to reproduce **

  1. mosquitto_pub -t command/$mac/on -m '' works.
  2. mosquitto_pub -t command/$mac/off -m '' works.
  3. Change "prefixcommand" from the default of command to foo/command.
  4. mosquitto_pub -t command/$mac/on -m '' does not work, as expected.
  5. mosquitto_pub -t foo/command/$mac/on -m '' does not work.

Does $mac come from my DHCP server? I don't seem to be able to change this string that gets embedded into topics from the Faikin end :-/

Expected behavior

mosquitto_pub -t foo/command/$mac/on -m '' should work, or if it is not going to work, the config page should reject it from being set.

The reason I want this is because I have quite a bit going on in MQTT, so I have separation of various subsystems by prefix and managed by ACL. So I'd like everything Faikin under a single prefix, ideally faikin, and then the ACL for the credentials that should be able to interact with Faikin would be simply faikin/#. "prefixapp" sounds like it would do the right thing, but the command/setting/state/info/error prefixes seem to go before that prefix instead, making this more of a pain to manage. It looked to me like making "prefixcommand" faikin/command should work, but it didn't.

Incidentally there's a separate top level Faikin topic that gets published to with more data that I cannot seem to change either, but I can cope with that.

basak avatar Apr 07 '24 16:04 basak

I am sure the documentation explains the prefix settings cannot be more than one part.

It splits the topic by / and then checks for the prefix command matching.

It cannot be xxx/yyy.

revk avatar Apr 07 '24 16:04 revk

OK, but then the configuration page should reject such a setting, surely? So there's that, but also please consider the feature request to support this for the reasons above.

basak avatar Apr 07 '24 19:04 basak

It's an issue with my library for this, not Faikin specific. The settings code has been improved over time, but does not have string validation. The current way it works means it splits up the topic first, then checks and also passes to a callback in the app. This would be a change to that logic and make it a tad I consistent with a special case for those prefixes only. Messy. Not impossible. The other change that has been asked for is the device name first then the message type afterwards, but that too means breaking up the topic first I expect.

Maybe raise an issue of the RevK project.

However I have checked and the description of the settings did make this clear, I thought.

image

revk avatar Apr 08 '24 06:04 revk

This needs to be a RevK library issue.

revk avatar May 06 '24 08:05 revk

This may work in latest beta, please test.

revk avatar May 06 '24 10:05 revk