Misc cleanups.

- Create dicts at compile time.
- Use startswith as it's less error prone.
- Add some FIXMEs.
- Make dict at compile time.
- Use enumerate.
- Lift sum out of if blocks.

src/Cargo.py

@@ -2,13 +2,6 @@
 
 class EmonHubCargo(object):
     uri = 0
-    timestamp = 0.0
-    target = 0
-    nodeid = 0
-    nodename = False
-    names = []
-    realdata = []
-    rssi = 0
 
     # The class "constructor" - It's actually an initializer
     def __init__(self, timestamp, target, nodeid, nodename, names, realdata, rssi, rawdata):
@@ -31,12 +24,4 @@ def __init__(self, timestamp, target, nodeid, nodename, names, realdata, rssi, r
         # self.realdatacodes = []
 
 def new_cargo(rawdata="", nodename=False, names=[], realdata=[], nodeid=0, timestamp=0.0, target=0, rssi=0.0):
-    """
-
-    :rtype : object
-    """
-
-    if not timestamp:
-        timestamp = time.time()
-    cargo = EmonHubCargo(timestamp, target, nodeid, nodename, names, realdata, rssi, rawdata)
-    return cargo
+    return EmonHubCargo(timestamp or time.time(), target, nodeid, nodename, names, realdata, rssi, rawdata)

src/emonhub.py

@@ -35,6 +35,7 @@
     if name != "__init__":
         # print "Loading: " + name
         setattr(ehi, name, getattr(getattr(namespace, name), name))
+del name
 
 """class EmonHub
 

src/emonhub_interfacer.py

@@ -39,7 +39,7 @@ def wrapper(*args):
 
 class EmonHubInterfacer(threading.Thread):
     def __init__(self, name):
-        # Initialize logger
+        # Initialise logger
         self._log = logging.getLogger("EmonHub")
 
         # Initialise thread
@@ -61,7 +61,7 @@ def __init__(self, name):
         self.init_settings = {}
         self._settings = {}
 
-        # Initialize message queue
+        # Initialise message queue
         self._sub_channels = {}
         self._pub_channels = {}
 
@@ -106,7 +106,7 @@ def run(self):
                         for channel in self._settings["pubchannels"]:
                             self._log.debug(str(rxc.uri) + " Sent to channel(start)' : " + str(channel))
 
-                            # Initialize channel if needed
+                            # Initialise channel if needed
                             if channel not in self._pub_channels:
                                 self._pub_channels[channel] = []
 
@@ -141,6 +141,7 @@ def add(self, cargo):
         try:
             f.append(cargo.timestamp)
             f.append(cargo.nodeid)
+            # FIXME replace with f.extend(cargo.realdata)
             for i in cargo.realdata:
                 f.append(i)
             if cargo.rssi:
@@ -218,12 +219,10 @@ def flush(self):
             if self._process_post(databuffer):
                 # In case of success, delete sample set from buffer
                 self.buffer.discardLastRetrievedItems(retrievedlength)
-                # log the time of last succesful post
-                self._interval_timestamp = time.time()
-            else:
-                # slow down retry rate in the case where the last attempt failed
-                # stops continuous retry attempts filling up the log
-                self._interval_timestamp = time.time()
+            # log the time of last successful post
+            # slow down retry rate in the case where the last attempt failed
+            # stops continuous retry attempts filling up the log
+            self._interval_timestamp = time.time()
 
 
     def _process_post(self, data):

src/emonhub_setup.py

@@ -9,8 +9,8 @@
 
 import time
 import logging
-from configobj import ConfigObj
 import json
+from configobj import ConfigObj
 
 """class EmonHubSetup
 

src/interfacers/EmonHubBMWInterfacer.py

@@ -46,7 +46,7 @@ def __init__(self, name, bmwapiusername='', bmwapipassword='', tempcredentialfil
         self._TempCredentialFile = tempcredentialfile
         self._first_time_loop = True
 
-        if os.path.exists(self._TempCredentialFile):
+        if os.path.exists(self._TempCredentialFile):  # FIXME race condition
             with open(self._TempCredentialFile, "r") as cf:
                 credentials = json.load(cf)
 
@@ -91,7 +91,7 @@ def obtainCredentials(self):
                 access_token = parts[0].split("#")
                 for word in access_token[1:]:
                     values = word.split("=")
-                    d[values[0]] = values[1]
+                    d[values[0]] = values[1]  # FIXME dict comprehension
 
                 # We should now have a dictionary object with three entries
                 # token_type, access_token, expires_in
@@ -131,7 +131,6 @@ def close(self):
     def _reset_duration_timer(self):
         """Reset timer to current date/time"""
         self._last_time_reading = time.time()
-        return
 
     def _is_it_time(self):
         """Checks to see if the duration has expired

src/interfacers/EmonHubEmoncmsHTTPInterfacer.py

@@ -121,7 +121,7 @@ def set(self, **kwargs):
                 # Next line will log apikey if uncommented (privacy ?)
                 #self._log.debug(self.name + " apikey: " + str(setting))
                 continue
-            elif key == 'url' and setting[:4] == "http":
+            elif key == 'url' and setting.startswith("http"):
                 self._log.info("Setting " + self.name + " url: " + setting)
                 self._settings[key] = setting
                 continue

src/interfacers/EmonHubJeeInterfacer.py

@@ -12,24 +12,21 @@
 
 class EmonHubJeeInterfacer(ehi.EmonHubSerialInterfacer):
 
-    def __init__(self, name, com_port='/dev/ttyAMA0', com_baud=0):
+    def __init__(self, name, com_port='/dev/ttyAMA0', com_baud=38400):
         """Initialize Interfacer
 
         com_port (string): path to COM port
 
         """
 
         # Initialization
-        if com_baud != 0:
-            super(EmonHubJeeInterfacer, self).__init__(name, com_port, com_baud)
-        else:
-            super(EmonHubJeeInterfacer, self).__init__(name, com_port, 38400)
+        super().__init__(name, com_port, 38400)
 
         # Display device firmware version and current settings
         self.info = ["", ""]
         if self._ser is not None:
             self._ser.write("v")
-            time.sleep(2)
+            time.sleep(2)  # FIXME sleep in initialiser smells
             self._rx_buf = self._rx_buf + self._ser.readline()
             if '\r\n' in self._rx_buf:
                 self._rx_buf = ""
@@ -137,7 +134,7 @@ def read(self):
             try:
                 c.rssi = int(r)
             except ValueError:
-                self._log.warning("Packet discarded as the RSSI format is invalid: "+ str(f))
+                self._log.warning("Packet discarded as the RSSI format is invalid: " + str(f))
                 return
             f = f[:-1]
 
@@ -172,7 +169,7 @@ def set(self, **kwargs):
                 setting = self._jee_settings[key]
             # convert bools to ints
             if str(setting).capitalize() in ['True', 'False']:
-                setting = int(setting == "True")
+                setting = int(setting.capitalize() == "True")
             # confirmation string always contains baseid, group and freq
             if all(i in self.info[1] for i in (" i", " g", " @ ", " MHz")):
                 # If setting confirmed as already set, continue without changing

src/interfacers/EmonHubMqttInterfacer.py

@@ -95,9 +95,9 @@ def _process_post(self, databuffer):
             try:
                 self._mqttc.username_pw_set(self.init_settings['mqtt_user'], self.init_settings['mqtt_passwd'])
                 self._mqttc.connect(self.init_settings['mqtt_host'], self.init_settings['mqtt_port'], 60)
-            except:
+            except Exception:
                 self._log.info("Could not connect...")
-                time.sleep(1.0)
+                time.sleep(1.0)  # FIXME why sleep? we're just about to return True
 
         else:
             frame = databuffer[0]

src/interfacers/EmonHubPacketGenInterfacer.py

@@ -1,3 +1,6 @@
+import time
+import requests
+from Cargo import new_cargo
 from emonhub_interfacer import EmonHubInterfacer
 
 """class EmonHubPacketGenInterfacer
@@ -111,8 +114,6 @@ def action(self):
 
             self._interval_timestamp = t
 
-        return
-
     def set(self, **kwargs):
         """
 
@@ -127,7 +128,7 @@ def set(self, **kwargs):
             if key in self._settings and self._settings[key] == setting:
                 continue
             elif key == 'apikey':
-                if setting[:4].lower() == 'xxxx':  # FIXME compare whole string to 'x'*32?
+                if setting.lower().startswith('xxxx'):  # FIXME compare whole string to 'x'*32?
                     self._log.warning("Setting " + self.name + " apikey: obscured")
                 elif len(setting) == 32:
                     self._log.info("Setting " + self.name + " apikey: set")
@@ -140,7 +141,7 @@ def set(self, **kwargs):
                 # Next line will log apikey if uncommented (privacy ?)
                 #self._log.debug(self.name + " apikey: " + str(setting))
                 continue
-            elif key == 'url' and setting[:4] == "http":
+            elif key == 'url' and setting.startswith("http"):
                 self._log.info("Setting " + self.name + " url: " + setting)
                 self._settings[key] = setting
                 continue

src/interfacers/EmonHubSMASolarInterfacer.py

@@ -141,7 +141,7 @@ def _increment_packet_send_counter(self):
 
         #Prevent roll over
         if self._packet_send_counter >= 0x0FFF:
-            self._packet_send_counter = 0
+            self._packet_send_counter = 0  # FIXME is this the same as "_reset_packet_send_counter" or not?
 
         #Appears that comms hangs on certain numbers
         #simple reset for now to resolve
@@ -203,68 +203,63 @@ def read(self):
             if self._btSocket is None:
                 return
 
-            readingsToMake = {}
-
-            readingsToMake["EnergyProduction"] = [0x54000200, 0x00260100, 0x002622FF]
-
-            #This causes problems with some inverters
-            #readingsToMake["SpotDCPower"] = [0x53800200, 0x00251E00, 0x00251EFF]
-
-            readingsToMake["SpotACPower"] = [0x51000200, 0x00464000, 0x004642FF]
-            readingsToMake["SpotACTotalPower"] = [0x51000200, 0x00263F00, 0x00263FFF]
-            readingsToMake["SpotDCVoltage"] = [0x53800200, 0x00451F00, 0x004521FF]
-            readingsToMake["SpotACVoltage"] = [0x51000200, 0x00464800, 0x004655FF]
-            readingsToMake["SpotGridFrequency"] = [0x51000200, 0x00465700, 0x004657FF]
-            readingsToMake["OperationTime"] = [0x54000200, 0x00462E00, 0x00462FFF]
-            readingsToMake["InverterTemperature"] = [0x52000200, 0x00237700, 0x002377FF]
-            #Not very useful for reporting
-            #readingsToMake["MaxACPower"] = [0x51000200, 0x00411E00, 0x004120FF]
-            #readingsToMake["MaxACPower2"] = [0x51000200, 0x00832A00, 0x00832AFF]
-            #readingsToMake["GridRelayStatus"] = [0x51800200, 0x00416400, 0x004164FF]
-
-            #Only useful on off grid battery systems
-            #readingsToMake["ChargeStatus"] = [0x51000200, 0x00295A00, 0x00295AFF]
-            #readingsToMake["BatteryInfo"] = [0x51000200, 0x00491E00, 0x00495DFF]
-
-
-            #Get first inverter in dictionary
-            inverter = self._Inverters[self._Inverters.keys()[0]]
+            readingsToMake = {
+                "EnergyProduction": [0x54000200, 0x00260100, 0x002622FF],
+
+                #This causes problems with some inverters
+                #readingsToMake["SpotDCPower"] = [0x53800200, 0x00251E00, 0x00251EFF]
+
+                "SpotACPower": [0x51000200, 0x00464000, 0x004642FF],
+                "SpotACTotalPower": [0x51000200, 0x00263F00, 0x00263FFF],
+                "SpotDCVoltage": [0x53800200, 0x00451F00, 0x004521FF],
+                "SpotACVoltage": [0x51000200, 0x00464800, 0x004655FF],
+                "SpotGridFrequency": [0x51000200, 0x00465700, 0x004657FF],
+                "OperationTime": [0x54000200, 0x00462E00, 0x00462FFF],
+                "InverterTemperature": [0x52000200, 0x00237700, 0x002377FF],
+                #Not very useful for reporting
+                #"MaxACPower": [0x51000200, 0x00411E00, 0x004120FF],
+                #"MaxACPower2": [0x51000200, 0x00832A00, 0x00832AFF],
+                #"GridRelayStatus": [0x51800200, 0x00416400, 0x004164FF],
+
+                #Only useful on off grid battery systems
+                #"ChargeStatus": [0x51000200, 0x00295A00, 0x00295AFF],
+                #"BatteryInfo": [0x51000200, 0x00491E00, 0x00495DFF],
+            }
+
+            #Get first inverter in dictionary  # FIXME dicts aren't sorted, there is no "first"
+            inverter = self._Inverters[list(self._Inverters.keys())[0]]
             self._log.debug("Reading from inverter " + inverter["inverterName"])
 
-
             #Loop through dictionary and take readings, building "output" dictionary as we go
             output = {}
-            for key in readingsToMake:
-
-                data = SMASolar_library.request_data(self._btSocket,
+            for reading, command in readingsToMake.items():
+                data = SMASolar_library.request_data(
+                    self._btSocket,
                     self._packet_send_counter,
                     self.mylocalBTAddress,
                     self.MySerialNumber,
-                    readingsToMake[key][0],
-                    readingsToMake[key][1],
-                    readingsToMake[key][2],
+                    command[0],
+                    command[1],
+                    command[2],
                     inverter["susyid"],
                     inverter["serialNumber"])
 
                 self._increment_packet_send_counter()
                 if data is not None:
                     output.update(SMASolar_library.extract_data(data))
                     if self._packettrace:
-                        self._log.debug("Packet reply for " + key + ". Packet from {0:04x}/{1:08x}".format(data.getTwoByte(14), data.getFourByteLong(16)))
+                        self._log.debug("Packet reply for " + reading + ". Packet from {0:04x}/{1:08x}".format(data.getTwoByte(14), data.getFourByteLong(16)))
                         self._log.debug(data.debugViewPacket())
 
-            #Sort the output to keep the keys in a consistent order
-            names = []
-            values = []
-            for key in sorted(output):
-                names.append(output[key].Label)
-                values.append(output[key].Value)
-
             #self._log.debug("Building cargo")
             c = Cargo.new_cargo()
             c.rawdata = None
-            c.realdata = values
-            c.names = names
+            #Sort the output to keep the keys in a consistent order
+            c.names = []
+            c.realdata = []
+            for key in sorted(output):
+                c.names.append(output[key].Label)
+                c.realdata.append(output[key].Value)
 
             #TODO: We need to revisit this once we know how multiple inverters communicate with us
             c.nodeid = inverter["NodeId"]

src/interfacers/EmonHubSerialInterfacer.py

@@ -1,5 +1,4 @@
 import serial
-import time
 from emonhub_interfacer import EmonHubInterfacer
 
 import Cargo

src/interfacers/EmonHubSocketInterfacer.py

@@ -138,7 +138,7 @@ def set(self, **kwargs):
             if key in self._settings and self._settings[key] == setting:
                 continue
             elif key == 'apikey':
-                if setting[:4].lower() == 'xxxx':  # FIXME compare whole string to 'x'*32?
+                if setting.lower().startswith('xxxx'):  # FIXME compare whole string to 'x'*32?
                     self._log.warning("Setting " + self.name + " apikey: obscured")
                 elif len(setting) == 32:
                     self._log.info("Setting " + self.name + " apikey: set")
@@ -151,7 +151,7 @@ def set(self, **kwargs):
                 # Next line will log apikey if uncommented (privacy ?)
                 #self._log.debug(self.name + " apikey: " + str(setting))
                 continue
-            elif key == 'url' and setting[:4] == "http":
+            elif key == 'url' and setting.startswith("http"):
                 self._log.info("Setting " + self.name + " url: " + setting)
                 self._settings[key] = setting
                 continue