revk / ESP32-Faikin

ESP32 based module to control Daikin aircon units
GNU General Public License v3.0
423 stars 63 forks source link

[FEATURE] Add a bulk alternative to command/<unit>/send #486

Open sm-Fifteen opened 1 week ago

sm-Fifteen commented 1 week ago

Is your feature request related to a problem? Please describe. Me and the guys over at #408 are trying to decypher the meaning of various S21 commands, and that often includes trying a bunch of them and tracking how they change based on the state of the system. For example, collect_profile.py has to pause between messages to avoid overwhelming the Faikin controller and losing messages as a result, but that wouldn't be neccessary if we could just send a list of commands for Faikin to attempt. The MQTT round-trip time also makes this inconveiniently slow, where it takes about 20-30 seconds to cycle through all messages my unit supports. The debug dump message exists, but only includes a limited set of known commands, and isn't well suited to exploratory analysis.

Describe the solution you'd like It would also be conveinient if we had an alternative to /send that allowed up to test multiple commands in bulk, maybe by sending a JSON array like ["F1\xAB\xCD89", "F8", "MM", "Rg", "Rz1"] (notice the use of non-ASCII escape sequences) and getting back a dict with each command requested being a key and each value having a similar format to what /send currently returns, so something like {"F1\xAB\xCD89": {"nak": true}, "F8": {"dump": "..."}} and the like.

Perhaps this would even be better suited to an HTTP route instead of an MQTT message since there isn't any reason why any device other than the one that sent the query would need to know and see the result, and HTTP clearly identifies the return value as the result of this specific query rather than some query that would have caused this message to be sent.

EDIT: I'm assuming that all commands in the bulk JSON array would be executed in sequence, but disregard whether each one succeeded or not, as this is for command debugging and not really for writing "S21 scripts" or whatever. That said, having them be executed in sequence would be useful to see what fields are affected by commands that change the internal state of the AC unit. There is also an open question of whether accessing this route should require admin password, or just be turned off by default unless enabled in the admin settings and be otherwise allowed without passwords (somewhat like how the Android Debug Bridge functions).

revk commented 1 week ago

I see why you want this, I'll see what I can do (busy with other work for a bit).

Sonic-Amiga commented 4 days ago

After getting some hands-on experience with the script... It's slow not because it has to wait 0.5 secs after each command; but because there is only one injected command per polling loop iteration. And the loop is simply too long, feels like 1.5 - secs. It's 2400 after all. So indeed would be nice to be able to send a packet job and get a packet of responses.

revk commented 4 days ago

Yeh, it just needs storing so the polling loop can stop, do the packets and get responses, and then continue. Should not be too hard to do.

sm-Fifteen commented 3 days ago

After getting some hands-on experience with the script... It's slow not because it has to wait 0.5 secs after each command; but because there is only one injected command per polling loop iteration. And the loop is simply too long, feels like 1.5 - secs. It's 2400 after all. So indeed would be nice to be able to send a packet job and get a packet of responses.

Well, there's a comment in the script that mentions the pause being there to make sure messages don't get dropped, so I went off of that, but didn't check myself. Interesting that the reason for it is that custom commands get queued to the end of the Faikin polling loop.

The point is that it's difficult to debug signals over MQTT both because of long delays for each signal sent (making it tedious to poll 20 at once to keep track of how they change and react to things) and the possibility of cross-talk (I can break my scripts if I'm sending MQTT commands over the same topics).

Also, 2400 bauds isn't that slow! It's like 6 times faster than HDMI-CEC! ;)

revk commented 2 days ago

Would it be better to have hexadecimal for payload?

revk commented 2 days ago

And are we happy to replace existing system for one that takes a set of JSON request:payload fields so even a single one is handled the same way, ideally hex?

revk commented 2 days ago

Ok, maybe... object not array, and allowing tag:value where tag is ASCII, and value is hex, and combined to be the message we send, but tag:null is simple text request with no payload, and maybe we allow array of ascii tag instead. Reply, yes, tag: and either a response (hex?) or something to indicate nak/ack (maybe false/true) or payload (hex?)

revk commented 2 days ago

And documented!

sm-Fifteen commented 2 days ago

Would it be better to have hexadecimal for payload?

I don't think so, no. The vast majority of signals use ASCII characters, and JSON escape sequences are well standardized, so it's pretty trivial to make those if needed.

Ok, maybe... object not array, and allowing tag:value where tag is ASCII, and value is hex, and combined to be the message we send, but tag:null is simple text request with no payload, and maybe we allow array of ascii tag instead. Reply, yes, tag: and either a response (hex?) or something to indicate nak/ack (maybe false/true) or payload (hex?)

I was suggesting an object for the return payload because it makes the request-response mapping clear at a glance, although an array might be more practical to reflect the sequence order and maybe to be able to handle sending ghe same command more than once in one request. I'm not sure the latter is a very important use case for what we're doing, though, so an object would probably be fine.

I don't think having a variable field that goes "Rx": "1234" like we currently do is especially useful, since Faikin doesn't know all commands and it's clear by now that they're not all 2 bytes long. I don't see the benefit of splitting the queries that way either. I would just leave the splitting up to the consumers. Having both string and raw representations is useful, though.

As for the distinction between NAK, ACK, timeout and whatever else, I would just make it into a status field.

Something along those lines:

Request body

["F\u0000", "BA", "RB", "Az"]

Response body

{
    "F\u0000": {"status": "nak"},
    "BA": {"status": "ack", "reply": null}, // ACK, no reply
    "RB": {"status": "ack", "reply": "SB1", "raw_hex": "025342319903"},
    "Az": {"status": "ack", "reply": "", "raw_hex": "0203"}, // Somehow
    ...
}
revk commented 1 day ago

I may finally be catching up, I'll let you know what I come up with.