cleanup, refactor

include/LCDDisplay.hpp

@@ -14,6 +14,11 @@ class LCDDisplay {
     std::string _dev;
     char writebuf[LCD_LINES * LCD_COLS + 8];                       // 4x40 chars + 2x4 setcursor commands
     static constexpr bool dualDisp = (LCD_LINES * LCD_COLS > 80);  // is it dual display device or not, evauluated from the size at compile time
+
+    void open();
+    void write(const uint8_t* cmdbuf, uint8_t len);
+    void close();
+
     void writeLines(std::array<std::string, 4> lines);             // do not use
     // we avoid using these, use buffer (even if these are faster for small writes, perhaps a rewrite is in order)
     void writeStr(std::string str);
@@ -22,8 +27,6 @@ class LCDDisplay {
    public:
     LCDDisplay(std::string dev, uint32_t baud = 0010002);  // from termios-baud.h, B115200
     ~LCDDisplay();
-    void open();
-    void close();
 
     void clear();
     void setContrast(uint8_t contrast);
@@ -35,11 +38,10 @@ class LCDDisplay {
     void writeBuf(uint8_t row, uint8_t col, std::string str);
     void clearBuf();
     void flushBuf();
+    char bar(uint8_t char_id, double value);
 
     // returns custom char no. c, small enough to define in header
     constexpr char cc(uint8_t c) { return c % 8; }
-
-    char bar(uint8_t char_id, double value);
 };
 
 #endif

src/LCDDisplay.cpp

@@ -28,71 +28,79 @@ LCDDisplay::LCDDisplay(std::string dev, uint32_t baud) {
     this->clearBuf();
 
     // disable underline and block cursor
-    uint8_t cmdbuf[] = {254, 75, 254, 84};
-    ::write(_lcd, &cmdbuf, 4);
+    constexpr uint8_t cmdbuf[] = {254, 75, 254, 84};
+    ::write(_lcd, &cmdbuf, sizeof(cmdbuf));
 
     // clear display and set cursor home
     this->clear();
     this->close();
+    log(LOG::INF, "LCDDisplay", "Opened LCD port at \"{}\"", _dev);
 }
 
 LCDDisplay::~LCDDisplay() {
     this->close();
+    log(LOG::INF, "LCDDisplay", "Closed LCD port");
 }
 
 void LCDDisplay::open() {
-    _lcd = ::open(_dev.c_str(), O_RDWR | O_NOCTTY | O_NONBLOCK); // ::open calls the global open() instead of LCDDisplay::open()
+    if (_lcd < 0)
+        _lcd = ::open(_dev.c_str(), O_RDWR | O_NOCTTY | O_NONBLOCK); // ::open calls the global open() instead of LCDDisplay::open()
     if (_lcd < 0) {
         log(LOG::ERR, "LCDDisplay", "Error opening serial port \"{}\"", _dev);  // err
         exit(_lcd);
     }
-    // log(LOG::INF, "LCDDisplay", "Opened LCD at \"{}\"", _dev);
 }
 
 void LCDDisplay::close() {
-    if (_lcd >= 0) ::close(_lcd);
+    if (_lcd >= 0)
+        ::close(_lcd);
     _lcd = -1;
-    // log(LOG::INF, "LCDDisplay", "LCD port closed");
+}
+
+void LCDDisplay::write(const uint8_t* cmdbuf, uint8_t len) {
+    this->open();
+    ::write(_lcd, cmdbuf, len);
+    this->close();
 }
 
 void LCDDisplay::clear() {
-    uint8_t cmdbuf[] = {254, 88};
-    ::write(_lcd, &cmdbuf, 2);
+    constexpr uint8_t cmdbuf[] = {254, 88};
+    this->write(cmdbuf, sizeof(cmdbuf));
 }
 
 void LCDDisplay::setContrast(uint8_t contrast) {
     log(LOG::INF, "LCDDisplay::setContrast", "Set contrast to {}", contrast);
-    uint8_t cmdbuf[] = {254, 80, contrast};
-    ::write(_lcd, &cmdbuf, 3);
+    const uint8_t cmdbuf[] = {254, 80, contrast};
+    this->write(cmdbuf, sizeof(cmdbuf));
 }
 
 void LCDDisplay::setBacklight(bool enabled) {
     log(LOG::INF, "LCDDisplay::setBacklight", "Set backlight to {}", enabled);
-    uint8_t cmdbuf[] = {254, (uint8_t)(enabled ? 66 : 70), 255};
-    ::write(_lcd, &cmdbuf, enabled ? 3 : 2);
+    const uint8_t cmdbuf[] = {254, (uint8_t)(enabled ? 66 : 70), 255};
+    this->write(cmdbuf, enabled ? 3 : 2);
 }
 
 /* still doesnt work? */
 void LCDDisplay::setBacklight(uint8_t brightness) {
     log(LOG::INF, "LCDDisplay::setBacklight", "Set backlight brightness to {}", brightness);
-    uint8_t cmdbuf[] = {254, (uint8_t)(brightness ? 66 : 70), 255, brightness};  // 3rd char is timer, 4th is brightness
-    ::write(_lcd, &cmdbuf, brightness ? 4 : 2);
+    const uint8_t cmdbuf[] = {254, (uint8_t)(brightness ? 66 : 70), 255, brightness};  // 3rd char is timer, 4th is brightness
+    this->write(cmdbuf, brightness ? 4 : 2);
 }
 
 void LCDDisplay::setCursor(uint8_t row, uint8_t col) {
-    uint8_t cmdbuf[] = {254, 71, (uint8_t)(col + 1), (uint8_t)(row + 1)};
-    ::write(_lcd, &cmdbuf, 4);
+    const uint8_t cmdbuf[] = {254, 71, (uint8_t)(col + 1), (uint8_t)(row + 1)};
+    this->write(cmdbuf, sizeof(cmdbuf));
 }
 
 void LCDDisplay::setCustomChar(uint8_t index, std::array<uint8_t, 8> cchar) {
     uint8_t cmdbuf[] = {254, 78, index, 0, 0, 0, 0, 0, 0, 0, 0};
-    std::memcpy(&cmdbuf[3], &cchar, cchar.size());
-    ::write(_lcd, &cmdbuf, 11);
+    std::memcpy(&cmdbuf[3], &cchar, sizeof(cchar));
+    this->write(cmdbuf, sizeof(cmdbuf));
 }
 
 /* immediately writes string to the lcd at current cursor pos */
 void LCDDisplay::writeStr(std::string str) {
-    ::write(_lcd, str.c_str(), str.size());
+    this->write((uint8_t*)(str.c_str()), str.size());
 }
 
 /* immediately writes string to the lcd at given cursor pos */
@@ -101,19 +109,6 @@ void LCDDisplay::writeStr(uint8_t row, uint8_t col, std::string str) {
     this->writeStr(str);
 }
 
-void LCDDisplay::writeLines(std::array<std::string, 4> lines) {
-    for (auto& line : lines) {
-        if (line.size() > LCD_COLS) {
-            log(LOG::WRN, "LCDDisplay::writeLines", "Line {} too long! ({} chars)", line, line.size());
-            continue;
-        }
-        line.insert(line.end(), LCD_COLS - line.size(), ' ');
-    }
-
-    this->writeStr(0, 0, lines[0] + lines[1]);
-    this->writeStr(2, 0, lines[2] + lines[3]);
-}
-
 /* writes string to write buffer at calculated pos */
 void LCDDisplay::writeBuf(uint8_t row, uint8_t col, std::string str) {
     size_t len = std::min(str.size(), sizeof(writebuf) - col);
@@ -129,17 +124,15 @@ void LCDDisplay::clearBuf() {
 
 /* flushes writebuffer to lcd, hyperoptmized, flushes buffer and commands in a single write operation */
 void LCDDisplay::flushBuf() {
-    // we could skip all of this and set it in constructor if we were extra sure the lines wont ever exceed 40 chars
     constexpr uint8_t setDisp[] = {254, 71, 1, 1};  // set cursor to char 1, line 1
     std::memcpy(&writebuf[0], setDisp, 4);
     std::memcpy(&writebuf[2 * LCD_COLS + 4], setDisp, 4);
     writebuf[2 * LCD_COLS + 7] = 3;  // overwrite set cursor c1 l1 to c1 l3
-
-    this->open();
-    ::write(_lcd, writebuf, sizeof(writebuf));
-    this->close();
+    this->write((uint8_t*)writebuf, sizeof(writebuf));
 }
 
+// i have a whole flushbuf func to avoid too many writes, but im writing on every bar, need to rethink this.
+// we could have a separate pre-char buffer and put all the setCustomChar commands to that?
 char LCDDisplay::bar(uint8_t char_id, double value) {
     std::array<uint8_t, 8> b;
     b.fill(0b01110);