ihormelnyk/opentherm_library

Typo and misleading function names

gergaly opened this issue · 1 comments

There is a typo in a function name: sendRequestAync should be sendRequestAsync.

The following function names are misleading because the Slave status flag in the response doesn't have *enabled flags, it has *active flags:
isCentralHeatingEnabled -> isCentralHeatingActive
isHotWaterEnabled -> isHotWaterActive
isCoolingEnabled -> isCoolingActive

Although I think you are right that "sendRequestAync" is probably a typo, method bool OpenTherm::sendRequestAync(unsigned long request) is not asynchronous, only the receive part is asynchronous. Therefor I think this typo is quite fortunate and renaming the method to sendRequestAsync is not a good idea because we can now implement an alternative, actual asynchronous send method, to match the asynchronous receive that was already partially implemented in process(). I am trying to build such an asynchronous send method now. I do not have experience with programming the hardware timer interrupt that is required to execute every 500us. I am trying however and once I have something I will create a PR.

Having an asynchronous send will enable easier combining with other libraries, plus it enables lower energy consumption for battery powered implementations. Albeit at the cost of one hardware timer and the associated loss of PWM functionality on some pins. Asynchronous sending is however optional: applications that require PWM or the hardware timer and do not need the power saving, can still use the existing API.