From 5693332ea7c440ee150bece9d265d0f80709a624 Mon Sep 17 00:00:00 2001 From: Siwat Sirichai Date: Sat, 3 Feb 2024 15:59:19 +0700 Subject: [PATCH] add comments --- src/ise_display.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/ise_display.cpp b/src/ise_display.cpp index 6b61db0..a3e33da 100644 --- a/src/ise_display.cpp +++ b/src/ise_display.cpp @@ -18,6 +18,7 @@ void ISEDisplay::begin(DigitalInputCard *inputCard, DigitalOutputCard *outputCar this->registerTouchCallback(bindedHandleTouch); this->reset(); delay(1000); + //TODO : Will the light be on or off when the system is started?, You need to jump to its respective page this->jumpToPage(1); delay(100); this->updateLightGroupState(); @@ -34,14 +35,20 @@ void ISEDisplay::handleTouch(uint8_t page, uint8_t component, uint8_t touch_type case COMPONENT_STANDBY_OPEN_ALL_TOGGLE: if (touch_type != TOUCH_TYPE_RELEASE) break; + //TODO : Should you really jump to page 2 here? should't page jumping be handled reactivly? + // EX. if atleast one light is on, then jump to active page, else jump to standby page + // This will allow page to change correctly when the system is started and when controlled remotely which won't call handleTouch this->jumpToPage(2); break; case COMPONENT_STANDBY_LIGHT_TOGGLE: if (touch_type != TOUCH_TYPE_RELEASE) break; + //TODO : So this does nothing? Shouldn't it turn on the lights? updateLightGroupStatePageStandby(); break; case COMPONENT_STANDBY_AC_TOGGLE: + //TODO : What's the expexted behavior of standby? Does it enter standby when the lights are all off and ignore the AC + // or does it only enter standby when ac is also off and purifier is off, or is it a timed thing? if (touch_type != TOUCH_TYPE_RELEASE) break; this->climateCard->setMode(this->climateCard->getMode() == 0 ? 1 : 0); @@ -62,10 +69,12 @@ void ISEDisplay::handleTouch(uint8_t page, uint8_t component, uint8_t touch_type switch (component) { case COMPONENT_LIGHT_MASTER_BUTTON: + //TODO : this only update the display to match the light, it doesn't toggle the light. if (touch_type != TOUCH_TYPE_RELEASE) break; updateLightGroupStatePageDashboard(); break; + //TODO : can't this be done better with array lookup? case COMPONENT_LIGHT_ROW1_LEVEL0_TOUCHPOINT: if (touch_type != TOUCH_TYPE_RELEASE) break; @@ -146,6 +155,7 @@ void ISEDisplay::handleTouch(uint8_t page, uint8_t component, uint8_t touch_type break; setLightLevel(4, 3); break; + // TODO : Don't we have fan only mode too? can you really just switch between 0 and 1? case COMPONENT_AC_TOGGLE_BUTTON: if (touch_type != TOUCH_TYPE_RELEASE) break; @@ -162,6 +172,7 @@ void ISEDisplay::handleTouch(uint8_t page, uint8_t component, uint8_t touch_type if (touch_type != TOUCH_TYPE_RELEASE) break; uint8_t fan_speed = this->climateCard->getFanSpeed(); + // TODO : We have auto, low, mid, high right?, that's 0,1,2,3 a modulo operation of 3 only gives 0,1,2 this->climateCard->setFanSpeed((fan_speed + 1) % 3); break; case COMPONENT_AC_TEMP_DOWN_BUTTON: @@ -237,6 +248,7 @@ void ISEDisplay::updateDateTimeText(rtctime_t time) this->giveSerialMutex(); } +// TODO : Implement void ISEDisplay::updateWeather(uint8_t weather_code, float outside_temp) { } @@ -381,6 +393,7 @@ void ISEDisplay::updateAirPurifierState() void ISEDisplay::updateACState() { + //TODO : The cognitive complexity here is so high, maybe break up the method a bit? // Get the state uint8_t mode = this->climateCard->getMode();