Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ESP-IDF #6

Closed
Bascht74 opened this issue Jan 23, 2024 · 13 comments
Closed

Support ESP-IDF #6

Bascht74 opened this issue Jan 23, 2024 · 13 comments

Comments

@Bascht74
Copy link
Collaborator

Bascht74 commented Jan 23, 2024

If you are using an ESP32 and you are using bluetooth with it (like I do for the external temperature updates), it is all written over the place that you have to / really should use the ESP-IDF instead of the arduino IDF.

The used api supports that already:
SwiCago/HeatPump@3085196

@SwiCago said the following about that...

...ESP32 (is) faster, we can lower delay.
Use UART custom define pin when retry.

Could you support the ESP-IDF with your component as well?
That way the bluetooth part would work more stable and lower delays/quicker response might be possible...

@echavet
Copy link
Owner

echavet commented Jan 24, 2024

Hello Bascht74 and thanks for your message,
I think I'm gonna need time or help, or both to implement this because I'm not familiar with this architecture. I've just tested to compile a yaml with my sources and the framework set to:

esp32:
  board: esp32doit-devkit-v1
  framework:
    type: esp-idf 

if fails with:

src/esphome/components/cn105/cn105.h:109:5: error: 'HardwareSerial' does not name a type

Perhaps it can be done just by using an alternative to HardwareSerial just for the ESP32.
I will have a look soon. But if you have advice, you're welcome.

@Bascht74
Copy link
Collaborator Author

I am not a programmer, but I will look into it...

@Bascht74
Copy link
Collaborator Author

Bascht74 commented Jan 24, 2024

What I found so far:

Switching from your custom UART-Code to the ESPHome one could be the best way to get it all. Maybe it is not so difficult (I cannot assess), because the ESPHome guys wrote
If you want to integrate a device into ESPHome that uses this (UART) protocol you can pretty much use almost all Arduino-based code because ESPHome has a nice abstraction over the UART bus.
see: https://esphome.io/custom/uart.html
and: https://esphome.io/components/uart

example: https://github.com/martin3000/ESPhome/blob/master/vaillantx6.h
or: https://github.com/J-Pipe/HTD-Multi-Room-Audio-Control/blob/main/Code/mca66.h

that way it would be possible to use ESP32 (Arduino/ESP-IDF)/ESP8266(Ardunino), and ESPHome would do the "magic":

uart:
  tx_pin: xx
  rx_pin: xx
  baud_rate: 4800

more available functions:

// Set TX/RX pins
id(my_uart).set_tx_pin(InternalGPIOPin *tx_pin);
id(my_uart).set_rx_pin(InternalGPIOPin *rx_pin);
// RX buffer size
id(my_uart).set_rx_buffer_size(size_t rx_buffer_size);
// Stop bits
id(my_uart).set_stop_bits(uint8_t stop_bits);
// Data bits
id(my_uart).set_data_bits(uint8_t data_bits);
// Parity
id(my_uart).set_parity(UARTParityOptions parity);
// Baud rate
id(my_uart).set_baud_rate(uint32_t baud_rate);
// Baud rate
id(my_uart).get_baud_rate();

@echavet
Copy link
Owner

echavet commented Jan 25, 2024

Hi Bascht74,

thanks for your message.
I did what you've asked in a different branch for the moment.

I have changed the way to configure the UART for both frameworks to work. I have tested quickly on my esp8266 with the arduino framework. And I have compiled but not tested a version with esp-idf framework.

Could you test with this branch (uart_config_change) ?

You have to change the source in external_components this way:

external_components:
  - source: github://echavet/MitsubishiCN105ESPHome@uart_config_change
    refresh: 0s

And you now have to add something like this :

uart:
  id: HP_UART
  baud_rate: 2400
  tx_pin: 1
  rx_pin: 3

Here is an complete yaml example:

substitutions:
  name: "hpsejour"
  friendly_name: Climatisation Séjour

esphome:
  name: ${name}
  friendly_name: ${friendly_name}

esp32:
  board: esp32doit-devkit-v1
  framework:
    type: esp-idf     
    #type: arduino


uart:
  id: HP_UART
  baud_rate: 2400
  tx_pin: 0
  rx_pin: 3
  

# Enable logging
logger:  
  hardware_uart: UART0
  level: INFO
  logs:
    CN105Climate: WARN
    CN105: WARN
    climate: WARN
    sensor: WARN
    chkSum : INFO
    WRITE : WARN
    READ : WARN
    Header: INFO
    Decoder : WARN
    

# Enable Home Assistant API
api:
  services:    
    - service: set_remote_temperature
      variables:
        temperature: float
      then:
        - lambda: 'id(clim_sejour).set_remote_temperature(temperature);'

    - service: use_internal_temperature
      then:
        - lambda: 'id(clim_sejour).set_remote_temperature(0);'
  encryption:
    key: !secret encryption_key

ota:  
  password: !secret ota_pwd

wifi:
 
  networks:
  - ssid : !secret wifi_ssid3
    password: !secret wifi_password3
  - ssid: !secret wifi_ssid
    password: !secret wifi_password

  # Enable fallback hotspot (captive portal) in case wifi connection fails
  ap:
    ssid: "Heatpump-Sejour Fallback Hotspot"
    password: !secret ota_pwd

captive_portal:

  
time:
  - platform: homeassistant
    id: homeassistant_time

external_components:
  - source: github://echavet/MitsubishiCN105ESPHome@uart_config_change
    refresh: 0s


# Text sensors with general information.
text_sensor:
  # Expose ESPHome version as sensor.
  - platform: version
    name: ${name} ESPHome Version
  # Expose WiFi information as sensors.
  - platform: wifi_info
    ip_address:
      name: ${name} IP
    ssid:
      name: ${name} SSID
    bssid:
      name: ${name} BSSID

# Sensors with general information.
sensor:
  # Uptime sensor.
  - platform: uptime
    name: ${name} Uptime

  # WiFi Signal sensor.
  - platform: wifi_signal
    name: ${name} WiFi Signal
    update_interval: 60s

# Configuration pour l'objet 'climate'
climate:
  - platform: cn105  
    name: ${friendly_name}
    id: "clim_sejour"    
    update_interval: 10s         

@echavet
Copy link
Owner

echavet commented Jan 25, 2024

You reported SwiCago was saying esp32 was faster and could provide a way to reduce delays.

My implementation doesn't need to reduce delays because there is no delay. None of the original delays have been retained.
This is why the code if far more efficient.
SwiCago lib is sending requests to the heatpump and then waits a 1 or 2 seconds delay to let the time to the heatpump to publish it's complete response. I don't.
I send the request and just continue executing the esp loop() which reads the response when it comes....
But I liked the uart abstraction and the idea to make the component compatible with all frameworks.
But before I pull to main, I need some tests...

@Bascht74
Copy link
Collaborator Author

Great to hear about the efficieny...
I will test it right now...

@Bascht74
Copy link
Collaborator Author

Bascht74 commented Jan 25, 2024

If I set the global log level to verbose I get some errors:

logger:  
  level: VERBOSE
...
Compiling .pioenvs/klima-og-tim/src/esphome/components/cn105/hp_writings.o
In file included from src/esphome/components/api/proto.h:4,
                 from src/esphome/components/api/api_pb2.h:5,
                 from src/esphome/components/api/api_connection.h:4,
                 from src/esphome.h:3,
                 from src/esphome/components/cn105/Globals.h:2,
                 from src/esphome/components/cn105/cn105.h:2,
                 from src/esphome/components/cn105/hp_readings.cpp:1:
src/esphome/components/cn105/hp_readings.cpp: In member function 'bool CN105Climate::checkSum()':
src/esphome/components/cn105/hp_readings.cpp:70:28: error: too many arguments for format [-Werror=format-extra-args]
         ESP_LOGV("chkSum", "adding %02X to %02X --> ", this->storedInputData[i], processedCS, processedCS + this->storedInputData[i]);
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/esphome/core/log.h:72:36: note: in definition of macro 'ESPHOME_LOG_FORMAT'
 #define ESPHOME_LOG_FORMAT(format) format
                                    ^~~~~~
src/esphome/core/log.h:154:28: note: in expansion of macro 'esph_log_v'
 #define ESP_LOGV(tag, ...) esph_log_v(tag, __VA_ARGS__)
                            ^~~~~~~~~~
src/esphome/components/cn105/hp_readings.cpp:70:9: note: in expansion of macro 'ESP_LOGV'
         ESP_LOGV("chkSum", "adding %02X to %02X --> ", this->storedInputData[i], processedCS, processedCS + this->storedInputData[i]);
         ^~~~~~~~
src/esphome/components/cn105/hp_readings.cpp: In member function 'void CN105Climate::checkFanSettings(heatpumpSettings&)':
src/esphome/components/cn105/hp_readings.cpp:533:23: error: format '%i' expects argument of type 'int', but argument 5 has type 'esphome::optional<esphome::climate::ClimateFanMode>' [-Werror=format=]
         ESP_LOGD(TAG, "Fan mode is: %i", this->fan_mode);
                       ^~~~~~~~~~~~~~~~~
src/esphome/core/log.h:72:36: note: in definition of macro 'ESPHOME_LOG_FORMAT'
 #define ESPHOME_LOG_FORMAT(format) format
                                    ^~~~~~
src/esphome/core/log.h:152:28: note: in expansion of macro 'esph_log_d'
 #define ESP_LOGD(tag, ...) esph_log_d(tag, __VA_ARGS__)
                            ^~~~~~~~~~
src/esphome/components/cn105/hp_readings.cpp:533:9: note: in expansion of macro 'ESP_LOGD'
         ESP_LOGD(TAG, "Fan mode is: %i", this->fan_mode);
         ^~~~~~~~
Compiling .pioenvs/klima-og-tim/src/esphome/components/cn105/utils.o
Compiling .pioenvs/klima-og-tim/src/esphome/components/esp32/core.o
Compiling .pioenvs/klima-og-tim/src/esphome/components/esp32/gpio.o
cc1plus: some warnings being treated as errors
*** [.pioenvs/klima-og-tim/src/esphome/components/cn105/hp_readings.o] Error 1
========================= [FAILED] Took 81.01 seconds =========================

@Bascht74
Copy link
Collaborator Author

If I use your log settings, it compiles OK...

@Bascht74
Copy link
Collaborator Author

Bascht74 commented Jan 25, 2024

So far it works...
I have not found anything technical else so far...

Good thing seems to be that I don't get any of those messages anymore:

[22:54:42][W][component:214]: Component esphome.coroutine took a long time for an operation (0.30 s).
[22:54:42][W][component:215]: Components should block for at most 20-30ms.

... only if I compile with arduino...


Only checking the status of the climate.
I moved it to a new issue, because same problem with arduino...
#9

@echavet
Copy link
Owner

echavet commented Jan 26, 2024

"Components should block for at most 20-30ms."

I get these messages when my logs are set too verbose. But it is worth it, and I think this is not a "worrying" warning. As the loop is not delayed by unnecessary instructions (like delays).

I'm not surprised to see these messages disappear when using esp32.

Thanks for investigating

@Bascht74
Copy link
Collaborator Author

Great to hear.

Fixing the two compile errors might be easy:

src/esphome/components/cn105/hp_readings.cpp: In member function 'bool CN105Climate::checkSum()':
src/esphome/components/cn105/hp_readings.cpp:70:28: error: too many arguments for format [-Werror=format-extra-args]
         ESP_LOGV("chkSum", "adding %02X to %02X --> ", this->storedInputData[i], processedCS, processedCS + this->storedInputData[i]);
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/esphome/components/cn105/hp_readings.cpp: In member function 'void CN105Climate::checkFanSettings(heatpumpSettings&)':
src/esphome/components/cn105/hp_readings.cpp:533:23: error: format '%i' expects argument of type 'int', but argument 5 has type 'esphome::optional<esphome::climate::ClimateFanMode>' [-Werror=format=]
         ESP_LOGD(TAG, "Fan mode is: %i", this->fan_mode);
                       ^~~~~~~~~~~~~~~~~

@echavet
Copy link
Owner

echavet commented Jan 26, 2024

should be ok now.
big merge done too, because of the esp-idf support

@Bascht74
Copy link
Collaborator Author

Thx, closing the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants