joshbenner / esphome-daikin-s21

ESPHome Component for Daikin Mini-Splits using the S21 serial port.
GNU General Public License v3.0
70 stars 22 forks source link

Issues with FTXC-D (and other units) #7

Open danijelt opened 1 year ago

danijelt commented 1 year ago

Follow up to issue in ESP32-Faikin: https://github.com/revk/ESP32-Faikin/issues/62.

The first problem is that some units don't have TX line inverted and they require level shifter because the unit won't respond to 3.3V signal on RX line. This can be solved by inverting the ESP's RX line and adding level shifter (or modifying the Faikin PCB as documented there in the new design), so it can just be noted in the README of this port.

The second, bigger problem, is that my FTXC60D replies to RH, RI, Ra and RL commands with NAK. And this ESPHome port heavily relies on temperature received from RH query to calculate offset and setpoint, making it unusable and causing all settings to reset to default (temperature set to 0 degrees, state off) on every status update because update() returns false

I was able to make it usable with this simple patch:

diff --git a/components/daikin_s21/climate/daikin_s21_climate.cpp b/components/daikin_s21/climate/daikin_s21_climate.cpp
index cc3cc30..7540e33 100644
--- a/components/daikin_s21/climate/daikin_s21_climate.cpp
+++ b/components/daikin_s21/climate/daikin_s21_climate.cpp
@@ -95,7 +95,7 @@ float DaikinS21Climate::get_effective_current_temperature() {
 }

 float DaikinS21Climate::get_room_temp_offset() {
-  if (!this->use_room_sensor()) {
+  if (this->use_room_sensor()) {
     return 0.0;
   }
   float room_val = this->room_sensor_degc();
diff --git a/components/daikin_s21/s21.cpp b/components/daikin_s21/s21.cpp
index 45d6e56..7c219fb 100644
--- a/components/daikin_s21/s21.cpp
+++ b/components/daikin_s21/s21.cpp
@@ -354,7 +354,7 @@ bool DaikinS21::run_queries(std::vector<std::string> queries) {
 }

 void DaikinS21::update() {
-  std::vector<std::string> queries = {"F1", "F5", "RH", "RI", "Ra", "RL", "Rd"};
+  std::vector<std::string> queries = {"F1", "F5", "Rd"};
   if (this->run_queries(queries) && !this->ready) {
     ESP_LOGI(TAG, "Daikin S21 Ready");
     this->ready = true;

This is a simple fix for my case where I have defined a temp sensor from HA, and the code is happy with it.

What would be the best approach for an universal fix? My idea is to add a config flag that would set set_supports_current_temperature to false and disable all calculations. Since this is just a convenient feature enabled by the link to Home Assistant, it shouldn't be a hard requirement. Or, alternatively, make adding external sensor mandatory if the unit doesn't respond to RH so we don't break the user's expectation about the look of a climate component?

joshbenner commented 1 year ago

The response to RH is used to set the controller's knowledge of what the Daikin itself thinks the temperature is. If my memory (and a quick glance at the code) serves me, the inside temperature reading from the Daikin is used in two ways:

  1. to control the Daikin if an external room temperature sensor is optionally used -- the controller will use the difference between the room sensor and the Daikin sensor to calculate an offset to apply to the setpoint.
  2. When reporting the current climate action when in "AUTO" (heat/cool) mode, since the Daikin does not indicate whether it is heating or cooling, the controller code assumes whether it is currently heating or cooling based on comparing the current temperature to the setpoint.

The patch you shared does 2 basic things: removes the queries your unit doesn't respond to, and disables room sensor offset calculation.

In the case of the room offset calculation -- if the inside temperature reported by the Daikin unit is unknown, then just do not use the room sensor feature until that can be fixed. Then the condition of this->use_room_sensor() will be false, and return the 0.0 offset and avoid adjusting the setpoint in an undesired way.

I think your suggestion about adding more config could be a reasonable workaround, though I think we'd need a flag that indicates something slightly different than just whether or not the integration supports the current temperature functionality provided by the HA integration (though that would be one of the results of such a flag).

Maybe something like:

I saw that revk stops sending a given query if it constantly NAKs -- I wonder if that would be a reasonable thing for this integration to do as well?

Obviously, the deeper issue is that the query codes in use are not obtaining the information we want from this Daikin unit. I think something like above is a reasonable workaround, though I can't help but wonder if there are some other codes we could send that unit which would report the temperature.

Do you know if the B045 adapter you mention in the Faikin issue reports current temperature readings from the unit? Do you have a B045 unit you could use to capture the queries it sends?

danijelt commented 1 year ago

Yes, I just bought BRP069B45 today and I'll try to capture the communication. Figuring out alternatives to RH and other queries seem more reasonable than crippling the code any further.

joshbenner commented 1 year ago

When I was integrating with my units, I was able to use the uart_mitm component by ssieb to see comms between my Daikin module (a *B041) and the mini-split. Perhaps you could do similar?

danijelt commented 1 year ago

Thanks a lot for the link. I modified it so that it doesn't work as MITM, only as a logger with the help of debug mode in ESPHome's UART.

Here's what I observed:

For other information, such as fan speed (RL), I'll try it later, maybe in the next few days.

MassiPi commented 1 year ago

probably we should try keeping trace of what each unit type manages, and how. For example, my FTXSxxG doesn't have any problem with 3.3V logic levels, but for examples:

i assume that the S21 is not that standard, after all.. phisically and logically

ddv2005 commented 1 year ago

I have FTXR units that don't have 5v on pin 5. And I made own cheap PCB design that works with external D1 Mini ESP32 WROOM boards and has read level shifter for both RX/TX lines without inversion. https://github.com/ddv2005/DaikinS21/tree/master/PCB/Daikin%203

image

danijelt commented 1 year ago

An update: I got it working with minimal modifications. For a quick fix, I modified the code to fetch inside and outside temperatures from F9 command instead of RH and Ra and it's working. There's a slight problem with this approach as some units don't report outside temperature in response to F9 (returns FF) so a rework of the update() would be needed to implement something like what Faikin does now, with retries and backoffs. Alternatively, support for reporting the outside temperature could be dropped as it seems that all models report at least inside temperature correctly in F9 query.

There's also an issue with what you call "fan speed", which is 999 in my case regardless of the AC's operating mode.

danijelt commented 1 year ago

I have created a pull request that fixes this issue - #11

dhbfischer commented 1 year ago

I've seen and tested pull request #11 with a FTXC35B and works fine. I think additional yaml options would be very nice (without backwards incompatible changes). I've seen the pull request from @ddv2005 and he's also adding query codes "F6" and "F7". The default codes for me didn't work and I have the same problem as @danijelt mentioned in the first comment. Why the defaults don't work on mine, I don't know. Could it possibly be a new API from Daikin? Mine is a FTXC35BV1B with year of manufacture 2020. In this case we could possibly define a configuration flag use_new_api or something. What do you think?

Also like @danijelt said in the first comment, I did use a TTL level converter from sparkfun in combination with a ESP32-S2 mini board which requires 3.3V instead of the given 5V. This works fine. Possibly the readme can be extended with a bit more information about how to connect different kind of boards.

joshbenner commented 1 year ago

I'm not convinced it's as simple as a new vs old API. I think various Daikin units support varied subsets of the commands & queries, and we just don't have a clear map of which units support which codes. As far as I've seen so far, a given command/query means the same thing everywhere -- so in that regard it seems the API is consistent.

I think there are a couple reasonable approaches to deal with this (and they do not exclude eachother):

  1. Try all the codes, tolerate failures, and disable codes that fail consistently (perhaps retrying them occasionally?)
  2. Allow the user to specify which codes to use with their unit, and only ever use those codes
dhbfischer commented 1 year ago

Good to know, in that case my preference is the first one. Try codes and disable them. In this case configuration stays simple and always as much as possible information is present.

amzaldua commented 1 year ago

Yes, I just bought BRP069B45 today and I'll try to capture the communication. Figuring out alternatives to RH and other queries seem more reasonable than crippling the code any further.

The BRP069B45 is compatible with a FTXM-R unit?

wrouesnel commented 11 months ago

As another data point: I'm running this code on FTXD80CV4 (using the code in #11) and it's working great - I have the input lines going straight to an M5Stack Atom with 10kOhm pull up resistors between the TX/RX and 5V lines. In this configuration, no UART lines need to be inverted and it seems to work just fine (I'm leaning on the 5V tolerance of the ESP chips on their GPIOs).