qix67 / comfortzone_heatpump

Arduino library to monitor and control Comfortzone EX50 heatpump
GNU General Public License v3.0
8 stars 5 forks source link

decouple platform-specific things from the main logic #6

Closed vinnyspb closed 1 year ago

vinnyspb commented 1 year ago

Hi @qix67,

As we discussed in a thread, I've came up with a way to make your library more platform-agnostic. With this approach it builds both on Arduino and ESP32 (including ESPHome).

In a nutshell, this PR decouples RS485 interface and other platform-dependent code from the main logic.

I've tested end-to-end on ESP32, but I don't have an Arduino device to test on, so for Arduino I only verified that it builds.

What do you think?

qix67 commented 1 year ago

Hi @vinnyspb,

I just tried to compile your pull request against my esp code and I have several warnings. Most (all?) of them seems related to an incorrect type of a parameter of write function.

Arduino: 1.8.13 (Linux), TD: 1.53, Board: "Generic ESP8266 Module, 80 MHz, Flash, Disabled, ck, 26 MHz, 40MHz, DOUT (compatible), 512K (no SPIFFS), 2, v2 Lower Memory, Disabled, None, Only Sketch, 115200"

Warning: Board Breadboard Arduino:avr:atmega328bb doesn't define a 'build.board' preference. Auto-set to: AVR_ATMEGA328BB
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from sketch/input_cmd.cpp:1:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:16,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30,
                 from sketch/this_uc_common.h:28,
                 from sketch/input_cmd.cpp:1:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from sketch/uc_settings.cpp:2:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:16,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30,
                 from sketch/this_uc_common.h:28,
                 from sketch/uc_settings.cpp:2:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from /xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino:1:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from sketch/domo_chauffe_eau.ino.cpp:1:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from sketch/wifi_com.cpp:1:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:16,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30,
                 from sketch/this_uc_common.h:28,
                 from sketch/wifi_com.cpp:1:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from sketch/htu21d.cpp:2:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:16,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30,
                 from sketch/this_uc_common.h:28,
                 from sketch/htu21d.cpp:2:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
/xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino: At global scope:
domo_chauffe_eau:26:22: error: no matching function for call to 'comfortzone_heatpump::comfortzone_heatpump()'
 comfortzone_heatpump heatpump;
                      ^
/xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino:26:22: note: candidates are:
In file included from sketch/this_uc_common.h:28:0,
                 from /xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino:1:
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:44:2: note: comfortzone_heatpump::comfortzone_heatpump(RS485Interface*)
  comfortzone_heatpump(RS485Interface* rs485) : rs485(rs485) {};
  ^
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:44:2: note:   candidate expects 1 argument, 0 provided
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:32:7: note: constexpr comfortzone_heatpump::comfortzone_heatpump(const comfortzone_heatpump&)
 class comfortzone_heatpump
       ^
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:32:7: note:   candidate expects 1 argument, 0 provided
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:32:7: note: constexpr comfortzone_heatpump::comfortzone_heatpump(comfortzone_heatpump&&)
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:32:7: note:   candidate expects 1 argument, 0 provided
/xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino: In function 'void setup()':
domo_chauffe_eau:93:39: error: no matching function for call to 'comfortzone_heatpump::begin(HardwareSerial&, int)'
  heatpump.begin(RS485SER, RS485_DE_PIN);
                                       ^
/xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino:93:39: note: candidate is:
In file included from sketch/this_uc_common.h:28:0,
                 from /xxxx/Arduino/domo_chauffe_eau/domo_chauffe_eau.ino:1:
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:46:7: note: void comfortzone_heatpump::begin()
  void begin();
       ^
/xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:46:7: note:   candidate expects 0 arguments, 2 provided
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from sketch/i2c.cpp:2:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:16,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30,
                 from sketch/this_uc_common.h:28,
                 from sketch/i2c.cpp:2:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
In file included from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30:0,
                 from sketch/this_uc_common.h:28,
                 from sketch/mqtt_com.cpp:1:
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h: In member function 'virtual int ArduinoRS485Interface::write_bytes(const void*, int)':
/xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:40:56: error: invalid conversion from 'const char*' to 'const uint8_t* {aka const unsigned char*}' [-fpermissive]
         return _hw_serial.write((const char*)data, size);
                                                        ^
In file included from /xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Arduino.h:263:0,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/rs485_interface.h:16,
                 from /xxxx/Arduino/libraries/comfortzone_heatpump/comfortzone_heatpump.h:30,
                 from sketch/this_uc_common.h:28,
                 from sketch/mqtt_com.cpp:1:
/xxxx/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/HardwareSerial.h:171:12: error:   initializing argument 1 of 'virtual size_t HardwareSerial::write(const uint8_t*, size_t)' [-fpermissive]
     size_t write(const uint8_t *buffer, size_t size) override
            ^
exit status 1
no matching function for call to 'comfortzone_heatpump::comfortzone_heatpump()'

This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.
vinnyspb commented 1 year ago

Thanks @qix67, I fixed the datatype error in 1f5f933781f60ff6c34faf6fdad39103f2d1092b. For whatever reason it builds for me either way.

There is one more error in your output which is related to the change I made in how comfortzone_heatpump object is initialized. Previously it had a default constructor with no parameters which assumed default Arduino's serial interface, and then you could override it by calling begin() method. I removed the default constructor and now it has to be initialized with a specific serial interface, see example.

begin() method still has to be called but without any parameters.

Let me know if you object to any of these changes, I'm happy to modify accordingly.

qix67 commented 1 year ago

Ok. It compiles properly now.

You have not updated API changes in README.txt file.

Once this is done, I will merge your pull request. Thanks.

vinnyspb commented 1 year ago

sure, updated README.txt now in a3a69f48c5ffac0ad964739b6d0b5de2d8e7f7b6

qix67 commented 1 year ago

Your pull request was just merged. Thanks for your contribution.

vinnyspb commented 1 year ago

did you actually merge? The PR looks closed, but not merged to me image

qix67 commented 1 year ago

Yes, I merged it using CLI command github gave. Maybe a display error occurs because original branch was already merged into develop this morning and I had to rebase your pull request to merge it.