From e9c7ab4cfc82172a07c19a0c4877b4afd11412f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Milants?= Date: Sat, 6 Nov 2021 19:01:19 +0100 Subject: [PATCH 1/6] Add data validity check and retries in CST816S driver. See https://github.com/InfiniTimeOrg/InfiniTime/issues/763#issuecomment-962436976. --- src/drivers/Cst816s.cpp | 73 +++++++++++++++++++++++++++++------------ src/drivers/Cst816s.h | 7 +++- 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/drivers/Cst816s.cpp b/src/drivers/Cst816s.cpp index 7fc8eca4..46dd96dc 100644 --- a/src/drivers/Cst816s.cpp +++ b/src/drivers/Cst816s.cpp @@ -32,6 +32,18 @@ bool Cst816S::Init() { twiMaster.Read(twiAddress, 0xa7, &dummy, 1); vTaskDelay(5); + static constexpr uint8_t maxRetries = 3; + bool isDeviceOk = false; + uint8_t retries = 0; + do { + isDeviceOk = CheckDeviceIds(); + retries++; + } while(!isDeviceOk && retries < maxRetries); + + if(!isDeviceOk) { + return false; + } + /* [2] EnConLR - Continuous operation can slide around [1] EnConUD - Slide up and down to enable continuous operation @@ -50,21 +62,7 @@ bool Cst816S::Init() { static constexpr uint8_t irqCtl = 0b01110000; twiMaster.Write(twiAddress, 0xFA, &irqCtl, 1); - // There's mixed information about which register contains which information - if (twiMaster.Read(twiAddress, 0xA7, &chipId, 1) == TwiMaster::ErrorCodes::TransactionFailed) { - chipId = 0xFF; - return false; - } - if (twiMaster.Read(twiAddress, 0xA8, &vendorId, 1) == TwiMaster::ErrorCodes::TransactionFailed) { - vendorId = 0xFF; - return false; - } - if (twiMaster.Read(twiAddress, 0xA9, &fwVersion, 1) == TwiMaster::ErrorCodes::TransactionFailed) { - fwVersion = 0xFF; - return false; - } - - return chipId == 0xb4 && vendorId == 0 && fwVersion == 1; + return true; } Cst816S::TouchInfos Cst816S::GetTouchInfo() { @@ -79,18 +77,33 @@ Cst816S::TouchInfos Cst816S::GetTouchInfo() { // This can only be 0 or 1 uint8_t nbTouchPoints = touchData[touchPointNumIndex] & 0x0f; - uint8_t xHigh = touchData[touchXHighIndex] & 0x0f; uint8_t xLow = touchData[touchXLowIndex]; - info.x = (xHigh << 8) | xLow; - + uint16_t x = (xHigh << 8) | xLow; uint8_t yHigh = touchData[touchYHighIndex] & 0x0f; uint8_t yLow = touchData[touchYLowIndex]; - info.y = (yHigh << 8) | yLow; + uint16_t y = (yHigh << 8) | yLow; + Gestures gesture = static_cast(touchData[gestureIndex]); + // Validity check + if(x >= maxX || y >= maxY || + (gesture != Gestures::None && + gesture != Gestures::SlideDown && + gesture != Gestures::SlideUp && + gesture != Gestures::SlideLeft && + gesture != Gestures::SlideRight && + gesture != Gestures::SingleTap && + gesture != Gestures::DoubleTap && + gesture != Gestures::LongPress)) { + info.isValid = false; + return info; + } + + info.x = x; + info.y = y; info.touching = (nbTouchPoints > 0); - info.gesture = static_cast(touchData[gestureIndex]); - + info.gesture = gesture; + info.isValid = true; return info; } @@ -108,3 +121,21 @@ void Cst816S::Wakeup() { Init(); NRF_LOG_INFO("[TOUCHPANEL] Wakeup"); } + +bool Cst816S::CheckDeviceIds() { + // There's mixed information about which register contains which information + if (twiMaster.Read(twiAddress, 0xA7, &chipId, 1) == TwiMaster::ErrorCodes::TransactionFailed) { + chipId = 0xFF; + return false; + } + if (twiMaster.Read(twiAddress, 0xA8, &vendorId, 1) == TwiMaster::ErrorCodes::TransactionFailed) { + vendorId = 0xFF; + return false; + } + if (twiMaster.Read(twiAddress, 0xA9, &fwVersion, 1) == TwiMaster::ErrorCodes::TransactionFailed) { + fwVersion = 0xFF; + return false; + } + + return chipId == 0xb4 && vendorId == 0 && fwVersion == 1; +} diff --git a/src/drivers/Cst816s.h b/src/drivers/Cst816s.h index 0fec8419..507dd4f5 100644 --- a/src/drivers/Cst816s.h +++ b/src/drivers/Cst816s.h @@ -21,7 +21,7 @@ namespace Pinetime { uint16_t y = 0; Gestures gesture = Gestures::None; bool touching = false; - bool isValid = true; + bool isValid = false; }; Cst816S(TwiMaster& twiMaster, uint8_t twiAddress); @@ -45,6 +45,8 @@ namespace Pinetime { return fwVersion; } private: + bool CheckDeviceIds(); + // Unused/Unavailable commented out static constexpr uint8_t gestureIndex = 1; static constexpr uint8_t touchPointNumIndex = 2; @@ -58,6 +60,9 @@ namespace Pinetime { //static constexpr uint8_t touchXYIndex = 7; //static constexpr uint8_t touchMiscIndex = 8; + static constexpr uint8_t maxX = 240; + static constexpr uint8_t maxY = 240; + TwiMaster& twiMaster; uint8_t twiAddress; From 8d61419836bffa7c59cb41c33fe747bafe841af5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Milants?= Date: Sun, 7 Nov 2021 16:19:06 +0100 Subject: [PATCH 2/6] Fix formatting following the code review. --- src/drivers/Cst816s.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/drivers/Cst816s.cpp b/src/drivers/Cst816s.cpp index 46dd96dc..4aac19f9 100644 --- a/src/drivers/Cst816s.cpp +++ b/src/drivers/Cst816s.cpp @@ -33,14 +33,14 @@ bool Cst816S::Init() { vTaskDelay(5); static constexpr uint8_t maxRetries = 3; - bool isDeviceOk = false; + bool isDeviceOk; uint8_t retries = 0; do { isDeviceOk = CheckDeviceIds(); retries++; - } while(!isDeviceOk && retries < maxRetries); + } while (!isDeviceOk && retries < maxRetries); - if(!isDeviceOk) { + if (!isDeviceOk) { return false; } From e6edf2155296864114a62cecbd0244c65c020a48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Milants?= Date: Sun, 7 Nov 2021 18:00:34 +0100 Subject: [PATCH 3/6] Disable the warning that is displayed when the initialization of the touch controller fails, as some users reported that it was displayed when a valid touch controller was installed. --- src/systemtask/SystemTask.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/systemtask/SystemTask.cpp b/src/systemtask/SystemTask.cpp index 0a3f9951..f2cc1dfb 100644 --- a/src/systemtask/SystemTask.cpp +++ b/src/systemtask/SystemTask.cpp @@ -144,9 +144,14 @@ void SystemTask::Work() { lcd.Init(); twiMaster.Init(); + /* + * TODO We disable this warning message until we ensure it won't be displayed + * on legitimate PineTime equipped with a compatible touch controller. + * (some users reported false positive). See https://github.com/InfiniTimeOrg/InfiniTime/issues/763 if (!touchPanel.Init()) { bootError = BootErrors::TouchController; } + */ dateTimeController.Register(this); batteryController.Register(this); motorController.Init(); From 76c43ebc82eb1d6580a10f292c83b0b18da135e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Milants?= Date: Sun, 7 Nov 2021 20:13:22 +0100 Subject: [PATCH 4/6] Fix previous commit, call touchPanel.Init() even if we disabled the touch controller boot error. --- src/systemtask/SystemTask.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/systemtask/SystemTask.cpp b/src/systemtask/SystemTask.cpp index f2cc1dfb..4b03f9ac 100644 --- a/src/systemtask/SystemTask.cpp +++ b/src/systemtask/SystemTask.cpp @@ -152,6 +152,7 @@ void SystemTask::Work() { bootError = BootErrors::TouchController; } */ + touchPanel.Init(); dateTimeController.Register(this); batteryController.Register(this); motorController.Init(); From 1d6455c28943efc0e51a91e6e7daa9045a372e50 Mon Sep 17 00:00:00 2001 From: Riku Isokoski Date: Tue, 9 Nov 2021 11:38:19 +0200 Subject: [PATCH 5/6] Fix Alarm app crashing on buttonpress --- src/displayapp/screens/Alarm.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/displayapp/screens/Alarm.h b/src/displayapp/screens/Alarm.h index 32a14d2f..487ba1d5 100644 --- a/src/displayapp/screens/Alarm.h +++ b/src/displayapp/screens/Alarm.h @@ -40,7 +40,9 @@ namespace Pinetime { Controllers::AlarmController& alarmController; lv_obj_t *time, *btnEnable, *txtEnable, *btnMinutesUp, *btnMinutesDown, *btnHoursUp, *btnHoursDown, *txtMinUp, *txtMinDown, - *txtHrUp, *txtHrDown, *btnRecur, *txtRecur, *btnMessage, *txtMessage, *btnInfo, *txtInfo; + *txtHrUp, *txtHrDown, *btnRecur, *txtRecur, *btnInfo, *txtInfo; + lv_obj_t* txtMessage = nullptr; + lv_obj_t* btnMessage = nullptr; enum class EnableButtonState { On, Off, Alerting }; void SetEnableButtonState(); From a57fda6ba4a29866083a1254ffdf92939d00e182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Milants?= Date: Thu, 11 Nov 2021 09:54:30 +0100 Subject: [PATCH 6/6] Set version to 1.7.0 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a8ecb81f..63257ff9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required(VERSION 3.10) -project(pinetime VERSION 1.6.0 LANGUAGES C CXX ASM) +project(pinetime VERSION 1.7.0 LANGUAGES C CXX ASM) set(CMAKE_C_STANDARD 99) set(CMAKE_CXX_STANDARD 14)