crankyoldgit / IRremoteESP8266

Infrared remote library for ESP8266/ESP32: send and receive infrared signals with multiple protocols. Based on: https://github.com/shirriff/Arduino-IRremote/
GNU Lesser General Public License v2.1
3k stars 833 forks source link

Refactor different Daikin protocols into separate files #1970

Open caliston opened 1 year ago

caliston commented 1 year ago

ir_Daikin.cpp is quite unwieldy, with about 4000 LOC. It covers several different Daikin IR protocols in a single file. That makes it hard to work on adding extra protocols because there's a lot of similar-but-different code for each one (some selected by #defines, some not).

This PR refactors that code so that each DaikinNNN protocol is in a separate file. The header file of ir_Daikin.h and tests remain unified, so it should make no difference to usage of the library.

Checked linting and passes all the tests.

caliston commented 1 year ago

Hmm, this is failing the supported-devices-check because that's expecting one header file per .cpp. It complains if the 'Supported' section is in the ir_Daikin.h header file because the new .cpps don't have a Supported section, but if I move that into the .cpp it complains it's not in the header file. I don't really want lots of ir_DaikinNNN.h because ir_Daikin.h is where most of the common code lives.

Any suggestions for the best approach? I suppose I could do something just for the purposes of passing the test, like having an ir_DaikinNNN.h just for the 'Supported' tags, but that wouldn't be very pleasant.

crankyoldgit commented 1 year ago

Hmm, this is failing the supported-devices-check because that's expecting one header file per .cpp. It complains if the 'Supported' section is in the ir_Daikin.h header file because the new .cpps don't have a Supported section, but if I move that into the .cpp it complains it's not in the header file. I don't really want lots of ir_DaikinNNN.h because ir_Daikin.h is where most of the common code lives.

Any suggestions for the best approach? I suppose I could do something just for the purposes of passing the test, like having an ir_DaikinNNN.h just for the 'Supported' tags, but that wouldn't be very pleasant.

Yeah, this is a hard one. I don't know the best approach either, otherwise it would have been refactored. Perhaps the better approach is to change how the check is performed to handle this case, and to ensure the scrape_supported_devices.py works too

caliston commented 1 year ago

Thanks, I'll take a look at that. Won't be able to do so for some weeks, but I'll put it on the todo list when I'm able to.