performance optimization

Dockerfile.dev

@@ -1,4 +1,4 @@
-FROM debian:stretch
+FROM debian:bullseye
 
 RUN apt update && apt install -y \
         curl \

config/inverter.conf

@@ -5,7 +5,7 @@
 #      /dev/ttyUSB0 if a USB<>Serial,
 #      /dev/hidraw0 if you're connecting via the USB port on the inverter.
 
-device=/dev/ttyUSB0
+device=/dev/hidraw0
 
 # How many times per hour is the program going to run...
 # This is used to calculate the PV & Load Watt Hours between runs...
@@ -20,23 +20,6 @@ amperage_factor=1.0
 
 # This allows you to modify the wattage in case the inverter is giving an incorrect
 # reading compared to measurement tools.  Normally this will remain '1'
-watt_factor=1.01
+#watt_factor=1.01
+watt_factor=1.0
 
-
-# The following settings allow you to modify runtime buffers.
-# N.B. These values may not be applicable to all inverter types, as such you will 
-# need to run docker exec -it voltronic-mqtt bash -c '/opt/inverter-cli/bin/inverter_poller -d -1'
-# or simply inverter_poller -d -1 to check for warnings or errors
-# mentioned in https://github.com/ned-kelly/docker-voltronic-homeassistant/issues/5
-
-# This allows you to modify the buffersize for the qpiri command
-qpiri=98
-
-# This allows you to modify the buffersize for the qpiws command
-qpiws=36
-
-# This allows you to modify the buffersize for the qmod command
-qmod=5
-
-# This allows you to modify the buffersize for the qpigs command
-qpigs=110

sources/healthcheck

@@ -1,9 +1,9 @@
 #!/bin/bash
 
-PROC=`ps cax | grep -E "mqtt-subscriber|mosquitto_sub|watch" | awk '{print $5}' | sort -u | wc -l`
+PROC=`ps cax | grep -E "mqtt-subscriber|mosquitto_sub" | awk '{print $5}' | sort -u | wc -l`
 
-if [ "$PROC" -eq "3" ] ; then
+if [ "$PROC" -eq "2" ] ; then
     exit 0
 else
     exit 99
-fi
+fi

sources/inverter-cli/inverter.cpp

@@ -9,16 +9,12 @@
 #include <fcntl.h>
 #include <termios.h>
 
-cInverter::cInverter(std::string devicename, int qpiri, int qpiws, int qmod, int qpigs) {
+cInverter::cInverter(std::string devicename) {
     device = devicename;
     status1[0] = 0;
     status2[0] = 0;
     warnings[0] = 0;
     mode = 0;
-    qpiri = qpiri;
-    qpiws = qpiws;
-    qmod = qmod;
-    qpigs = qpigs;
 }
 
 string *cInverter::GetQpigsStatus() {
@@ -68,19 +64,39 @@ int cInverter::GetMode() {
     return result;
 }
 
-bool cInverter::query(const char *cmd, int replysize) {
+#define HEX(x) (x < 10 ? ('0' + x) : ('a' + x - 10))
+char *cInverter::escape_strn(unsigned char *str, int n) {
+    int j=0;
+
+    for (int i=0; i<n; i++) {
+	if (isprint(str[i]))
+	    escaped_buf[j++] = str[i];
+	else {
+	    unsigned char x1 = str[i] >> 4;
+	    unsigned char x2 = str[i] & 0x0f;
+	    escaped_buf[j++] = '\\';
+	    escaped_buf[j++] = 'x';
+	    escaped_buf[j++] = HEX(x1);
+	    escaped_buf[j++] = HEX(x2);
+	}
+    }
+
+    escaped_buf[j] = '\0';
+    return escaped_buf;
+}
+
+bool cInverter::query(const char *cmd) {
     time_t started;
     int fd;
-    int i=0, n;
+    int i=0, n, write_failed=0;
 
     fd = open(this->device.data(), O_RDWR | O_NONBLOCK);
     if (fd == -1) {
-        lprintf("INVERTER: Unable to open device file (errno=%d %s)", errno, strerror(errno));
+        lprintf("Unable to open device file (errno=%d %s)", errno, strerror(errno));
         sleep(5);
         return false;
     }
 
-
     // Once connected, set the baud rate and other serial config (Don't rely on this being correct on the system by default...)
     speed_t baud = B2400;
 
@@ -105,54 +121,90 @@ bool cInverter::query(const char *cmd, int replysize) {
     uint16_t crc = cal_crc_half((uint8_t*)cmd, strlen(cmd));
     n = strlen(cmd);
     memcpy(&buf, cmd, n);
-    lprintf("INVERTER: Current CRC: %X %X", crc >> 8, crc & 0xff);
+    lprintf("%s: command CRC: %X %X", cmd, crc >> 8, crc & 0xff);
 
     buf[n++] = crc >> 8;
     buf[n++] = crc & 0xff;
-    buf[n++] = 0x0d;
+    buf[n++] = 0x0d; // '\r'
+    buf[n+1] = '\0'; // see workaround below
+
+    // send a command
+    int chunk_size = 8;
+    for (int offset = 0; offset < n; usleep(50000)) {
+	int left = n - offset;
+	int towrite = left > chunk_size ? chunk_size : left;
+	// WORKAROUND: For some reason, writing 1 byte causes it to error.
+	// However, since we padded with '\0' above, we can give it 2 instead.
+	// I don't know of any 6 (+ 2*CRC + '\r') byte commands to test it on
+	// but this at least gets it to return NAK.
+	if (towrite == 1) towrite = 2;
+        lprintf("%s: write offset %d, writing %d", cmd, offset, towrite);
+	ssize_t written = write(fd, &buf[offset], towrite);
+	if (written > 0)
+	    offset += written;
+	else {
+	    lprintf("%s: write failed (written=%d, errno=%d: %s)", cmd, written, errno, strerror(errno));
+	    write_failed=1;
+	    break;
+	}
+        lprintf("%s: %d bytes to write, %d bytes written", cmd, n, offset);
+    }
 
-    //send a command
-    write(fd, &buf, n);
+    // reads tend to be in multiple of 8 chars with NULL padding as necessary
+    char *startbuf = (char *)&buf[0];
+    char *endbuf = 0;
     time(&started);
-
     do {
-        n = read(fd, (void*)buf+i, replysize-i);
+        n = read(fd, &buf[i], 120-i);
         if (n < 0) {
             if (time(NULL) - started > 2) {
-                lprintf("INVERTER: %s read timeout", cmd);
+                lprintf("%s: read timeout", cmd);
                 break;
             } else {
                 usleep(10);
                 continue;
             }
         }
 
+	endbuf = (char *)memrchr((void *)&buf[i], 0x0d, n);
         i += n;
-    } while (i<replysize);
+    } while (endbuf == NULL);
     close(fd);
+    buf[i] = '\0';
 
-    if (i==replysize) {
+    lprintf("%s: %d bytes in reply: %s", cmd, i, escape_strn(buf, i));
 
-        lprintf("INVERTER: %s reply size (%d bytes)", cmd, i);
+    if (write_failed) {
+	return false;
+    }
+    else if (i < 3) {
+        lprintf("%s: reply too short (%d bytes)", cmd, i);
+	return false;
+    }
+    else if (endbuf == NULL) {
+        lprintf("%s: couldn't find reply <cr>: %s", cmd, buf);
+	return false;
+    }
 
-        if (buf[0]!='(' || buf[replysize-1]!=0x0d) {
-            lprintf("INVERTER: %s: incorrect start/stop bytes.  Buffer: %s", cmd, buf);
-            return false;
-        }
+    int replysize = endbuf - startbuf + 1;
+
+    // proper response, check CRC
+    if (buf[0]=='(' && buf[replysize-1]==0x0d) {
         if (!(CheckCRC(buf, replysize))) {
-            lprintf("INVERTER: %s: CRC Failed!  Reply size: %d  Buffer: %s", cmd, replysize, buf);
+            lprintf("%s: CRC failed!", cmd);
             return false;
         }
-
-        buf[i-3] = '\0'; //nullterminating on first CRC byte
-        lprintf("INVERTER: %s: %d bytes read: %s", cmd, i, buf);
-
-        lprintf("INVERTER: %s query finished", cmd);
-        return true;
-    } else {
-        lprintf("INVERTER: %s reply too short (%d bytes)", cmd, i);
+	replysize -= 3;
+        buf[replysize] = '\0'; // null terminating on first CRC byte
+    }
+    else {
+        lprintf("%s: incorrect start/stop bytes", cmd);
         return false;
     }
+
+    lprintf("%s: %d bytes in payload", cmd, replysize);
+    lprintf("%s: query finished", cmd);
+    return true;
 }
 
 void cInverter::poll() {
@@ -163,15 +215,17 @@ void cInverter::poll() {
 
         // Reading mode
         if (!ups_qmod_changed) {
-            if (query("QMOD", qmod)) {
+            if (query("QMOD") &&
+		strcmp((char *)&buf[1], "NAK") != 0) {
                 SetMode(buf[1]);
                 ups_qmod_changed = true;
             }
         }
 
         // reading status (QPIGS)
         if (!ups_qpigs_changed) {
-            if (query("QPIGS", qpigs)) {
+            if (query("QPIGS") &&
+		strcmp((char *)&buf[1], "NAK") != 0) {
                 m.lock();
                 strcpy(status1, (const char*)buf+1);
                 m.unlock();
@@ -181,7 +235,8 @@ void cInverter::poll() {
 
         // Reading QPIRI status
         if (!ups_qpiri_changed) {
-            if (query("QPIRI", qpiri)) {
+            if (query("QPIRI") &&
+		strcmp((char *)&buf[1], "NAK") != 0) {
                 m.lock();
                 strcpy(status2, (const char*)buf+1);
                 m.unlock();
@@ -191,7 +246,8 @@ void cInverter::poll() {
 
         // Get any device warnings...
         if (!ups_qpiws_changed) {
-            if (query("QPIWS", qpiws)) {
+            if (query("QPIWS") &&
+		strcmp((char *)&buf[1], "NAK") != 0) {
                 m.lock();
                 strcpy(warnings, (const char*)buf+1);
                 m.unlock();
@@ -205,7 +261,7 @@ void cInverter::poll() {
 
 void cInverter::ExecuteCmd(const string cmd) {
     // Sending any command raw
-    if (query(cmd.data(), 7)) {
+    if (query(cmd.data())) {
         m.lock();
         strcpy(status2, (const char*)buf+1);
         m.unlock();

sources/inverter-cli/inverter.h

@@ -7,7 +7,8 @@
 using namespace std;
 
 class cInverter {
-    unsigned char buf[1024]; //internal work buffer
+    unsigned char buf[1024];	// internal work buffer
+    char escaped_buf[4096];	// screen-printable version of above
 
     char warnings[1024];
     char status1[1024];
@@ -19,11 +20,12 @@ class cInverter {
 
     void SetMode(char newmode);
     bool CheckCRC(unsigned char *buff, int len);
-    bool query(const char *cmd, int replysize);
+    bool query(const char *cmd);
     uint16_t cal_crc_half(uint8_t *pin, uint8_t len);
+    char *escape_strn(unsigned char *str, int n);
 
     public:
-        cInverter(std::string devicename, int qpiri, int qpiws, int qmod, int qpigs);
+        cInverter(std::string devicename);
         void poll();
         void runMultiThread() {
             std::thread t1(&cInverter::poll, this);

sources/inverter-cli/main.cpp

@@ -43,7 +43,6 @@ string devicename;
 int runinterval;
 float ampfactor;
 float wattfactor;
-int qpiri, qpiws, qmod, qpigs;
 
 // ---------------------------------------
 
@@ -91,14 +90,6 @@ void getSettingsFile(string filename) {
                     attemptAddSetting(&wattfactor, linepart2);
                 else if(linepart1 == "watt_factor")
                     attemptAddSetting(&wattfactor, linepart2);
-                else if(linepart1 == "qpiri")
-                    attemptAddSetting(&qpiri, linepart2);
-                else if(linepart1 == "qpiws")
-                    attemptAddSetting(&qpiws, linepart2);
-                else if(linepart1 == "qmod")
-                    attemptAddSetting(&qmod, linepart2);
-                else if(linepart1 == "qpigs")
-                    attemptAddSetting(&qpigs, linepart2);
                 else
                     continue;
             }
@@ -170,7 +161,7 @@ int main(int argc, char* argv[]) {
     if(cmdArgs.cmdOptionExists("-1") || cmdArgs.cmdOptionExists("--run-once")) {
         runOnce = true;
     }
-    lprintf("INVERTER: Debug set");
+    lprintf("Debug set");
 
     // Get the rest of the settings from the conf file
     if( access( "./inverter.conf", F_OK ) != -1 ) { // file exists
@@ -180,7 +171,7 @@ int main(int argc, char* argv[]) {
     }
 
     bool ups_status_changed(false);
-    ups = new cInverter(devicename,qpiri,qpiws,qmod,qpigs);
+    ups = new cInverter(devicename);
 
     // Logic to send 'raw commands' to the inverter..
     if (!rawcmd.empty()) {
@@ -197,7 +188,7 @@ int main(int argc, char* argv[]) {
             int mode = ups->GetMode();
 
             if (mode)
-                lprintf("INVERTER: Mode Currently set to: %d", mode);
+                lprintf("Mode Currently set to: %d", mode);
 
             ups_status_changed = false;
         }
@@ -216,15 +207,15 @@ int main(int argc, char* argv[]) {
             if (reply1 && reply2 && warnings) {
 
                 // Parse and display values
-                sscanf(reply1->c_str(), "%f %f %f %f %d %d %d %d %f %d %d %d %f %f %f %d %s", &voltage_grid, &freq_grid, &voltage_out, &freq_out, &load_va, &load_watt, &load_percent, &voltage_bus, &voltage_batt, &batt_charge_current, &batt_capacity, &temp_heatsink, &pv_input_current, &pv_input_voltage, &scc_voltage, &batt_discharge_current, &device_status);
+                sscanf(reply1->c_str(), "%f %f %f %f %d %d %d %d %f %d %d %d %f %f %f %d %s", &voltage_grid, &freq_grid, &voltage_out, &freq_out, &load_va, &load_watt, &load_percent, &voltage_bus, &voltage_batt, &batt_charge_current, &batt_capacity, &temp_heatsink, &pv_input_current, &pv_input_voltage, &scc_voltage, &batt_discharge_current, (char *)&device_status);
                 sscanf(reply2->c_str(), "%f %f %f %f %f %d %d %f %f %f %f %f %d %d %d %d %d %d - %d %d %d %f", &grid_voltage_rating, &grid_current_rating, &out_voltage_rating, &out_freq_rating, &out_current_rating, &out_va_rating, &out_watt_rating, &batt_rating, &batt_recharge_voltage, &batt_under_voltage, &batt_bulk_voltage, &batt_float_voltage, &batt_type, &max_grid_charge_current, &max_charge_current, &in_voltage_range, &out_source_priority, &charger_source_priority, &machine_type, &topology, &out_mode, &batt_redischarge_voltage);
 
                 // There appears to be a discrepancy in actual DMM measured current vs what the meter is
                 // telling me it's getting, so lets add a variable we can multiply/divide by to adjust if
                 // needed.  This should be set in the config so it can be changed without program recompile.
                 if (debugFlag) {
-                    printf("INVERTER: ampfactor from config is %.2f\n", ampfactor);
-                    printf("INVERTER: wattfactor from config is %.2f\n", wattfactor);
+                    printf("ampfactor from config is %.2f\n", ampfactor);
+                    printf("wattfactor from config is %.2f\n", wattfactor);
                 }
 
                 pv_input_current = pv_input_current * ampfactor;
@@ -283,7 +274,7 @@ int main(int argc, char* argv[]) {
 
                 if(runOnce) {
                     // Do once and exit instead of loop endlessly
-                    lprintf("INVERTER: All queries complete, exiting loop.");
+                    lprintf("All queries complete, exiting loop.");
                     exit(0);
                 }
             }

sources/inverter-cli/tools.cpp

@@ -28,7 +28,7 @@ void lprintf(const char *format, ...) {
         buf[strlen(buf)-1] = 0;
 
         //connect with args
-        snprintf(fmt, sizeof(fmt), "%s %s\n", buf, format);
+        snprintf(fmt, sizeof(fmt), "%s INVERTER: %s\n", buf, format);
 
         //put on screen:
         va_start(ap, format);
@@ -71,4 +71,4 @@ int print_help() {
     printf("                            PEx / PDx (Enable/disable backlight)\n\n");
 
     return 1;
-}
+}