From 715bbaf85d81e94dd30f5db4fc20fc35aaf6228a Mon Sep 17 00:00:00 2001
From: Andre LaFleur <cincospenguinos@gmail.com>
Date: Sat, 18 Dec 2021 10:16:47 -0700
Subject: [PATCH] Begin RTC refactor

* My current understanding of the WatchyRTC class is that it came about
  due to the need to ship Watchys with new RTC chips. The class here is
  designed to accomodate both chips
* This class is a good way of doing just that--a single spot that
  hides the implementation details of both chips. The downside to this
  approach in the moment is that we have a bunch of conditionals that
  check if we have this or that chip, and if we ever need to support
  yet another chip, we will have an even more complex WatchyRTC class
* I am going to refactor this to simplify it so that way we will not
  have to worry about modifying this much if we need to support a new
  chip, or if someone else wants to support a new chip
---
 src/WatchyRTC.cpp | 14 +++++++-------
 src/WatchyRTC.h   | 20 ++++++++++++++++++--
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/WatchyRTC.cpp b/src/WatchyRTC.cpp
index 298c7b6..b4b0c49 100644
--- a/src/WatchyRTC.cpp
+++ b/src/WatchyRTC.cpp
@@ -8,12 +8,12 @@ void WatchyRTC::init(){
     Wire.beginTransmission(RTC_DS_ADDR);
     error = Wire.endTransmission();
     if(error == 0){
-        rtcType = DS3231;
+        rtcType = DS3231_RTC_TYPE;
     }else{
         Wire.beginTransmission(RTC_PCF_ADDR);
         error = Wire.endTransmission();
         if(error == 0){
-            rtcType = PCF8563;
+            rtcType = PCF8563_RTC_TYPE;
         }else{
             //RTC Error
         }
@@ -21,7 +21,7 @@ void WatchyRTC::init(){
 }
 
 void WatchyRTC::config(String datetime){
-    if(rtcType == DS3231){
+    if(rtcType == DS3231_RTC_TYPE){
         _DSConfig(datetime);
     }else{
         _PCFConfig(datetime);
@@ -29,7 +29,7 @@ void WatchyRTC::config(String datetime){
 }
 
 void WatchyRTC::clearAlarm(){
-    if(rtcType == DS3231){
+    if(rtcType == DS3231_RTC_TYPE){
         rtc_ds.alarm(ALARM_2);
     }else{
         int nextAlarmMinute = 0;
@@ -41,7 +41,7 @@ void WatchyRTC::clearAlarm(){
 }
 
 void WatchyRTC::read(tmElements_t &tm){
-    if(rtcType == DS3231){
+    if(rtcType == DS3231_RTC_TYPE){
         rtc_ds.read(tm);
         tm.Year = tm.Year - 30; //reset to offset from 2000
     }else{
@@ -61,7 +61,7 @@ void WatchyRTC::read(tmElements_t &tm){
 }
 
 void WatchyRTC::set(tmElements_t tm){
-    if(rtcType == DS3231){
+    if(rtcType == DS3231_RTC_TYPE){
         tm.Year = tm.Year + 2000 - YEAR_OFFSET_DS;
         time_t t = makeTime(tm);
         rtc_ds.set(t);
@@ -73,7 +73,7 @@ void WatchyRTC::set(tmElements_t tm){
 }
 
 uint8_t WatchyRTC::temperature(){
-    if(rtcType == DS3231){
+    if(rtcType == DS3231_RTC_TYPE){
         return rtc_ds.temperature();
     }else{
         return 255; //error
diff --git a/src/WatchyRTC.h b/src/WatchyRTC.h
index ea5350b..20975c4 100644
--- a/src/WatchyRTC.h
+++ b/src/WatchyRTC.h
@@ -4,13 +4,29 @@
 #include <DS3232RTC.h>
 #include <Rtc_Pcf8563.h>
 
-#define DS3231 0
-#define PCF8563 1
+#define DS3231_RTC_TYPE 0
+#define PCF8563_RTC_TYPE 1
 #define RTC_DS_ADDR 0x68
 #define RTC_PCF_ADDR 0x51
 #define YEAR_OFFSET_DS 1970
 #define YEAR_OFFSET_PCF 2000
 
+// TODO: So we're relying on an rtcType as a multiplexer, making our WatchyRTC code a bit
+// more complex. A way around this is to use a command pattern instead:
+// https://sourcemaking.com/design_patterns/command
+//
+// That way we don't have to have a bunch of conditionals floating around in this class.
+// It will simplify the process of adding a new RTC chip later as well.
+
+class AbstractRTC {
+public:
+    virtual void config(String datetime);
+    virtual void clearAlarm();
+    virtual void read(tmElements_t &tm);
+    virtual void set(tmElements_t tm);
+    virtual uint8_t temperature();
+};
+
 class WatchyRTC {
     public:
         DS3232RTC rtc_ds;