Writing a class to hold Arduino Properties for an Arduino I2C Debugger

Question

I have this class which mostly holds a bunch of properties and their setters. I also have wrappers for some functions of another module. Here is the class:

class ARCArduino(object):
 """
 Class to manage Arduino functionality
 """
 def __init__(self, tree_item, port, identifier):
 self.__i2c = ArduinoI2C()
 self.__i2c.setPort(port)
 self.__i2c.connectPort()
 self.__id = None
 self.__i2c_version = None
 self.__switch_version = None
 self.__tree_item = tree_item
 self.__address_list = []
 self.__port = port
 self.__identifier = identifier
 @property
 def id(self):
 if self.__id is None:
 try:
 self.__id = self.__i2c.getID()
 except IOError:
 self.__id = "NO CONNECTION"
 return self.__id
 @property
 def i2c_version(self):
 if self.__i2c_version is None:
 try:
 self.__i2c_version = self.__i2c.i2cVersion()
 except IOError:
 self.__i2c_version = "NO CONNECTION"
 return self.__i2c_version
 @property
 def switch_version(self):
 if self.__switch_version is None:
 try:
 self.__switch_version = self.__i2c.switchVersion()
 except IOError:
 self.__switch_version = "NO CONNECTION"
 return self.__switch_version
 @property
 def tree_item(self):
 return self.__tree_item
 @tree_item.setter
 def tree_item(self, item):
 if type(item) is not wx.TreeItemId:
 return
 self.__tree_item = item
 @property
 def address_list(self):
 return self.__address_list
 @address_list.setter
 def address_list(self, addr_list):
 if type(addr_list) is not list:
 return
 self.__address_list = addr_list
 @property
 def port(self):
 return self.__port
 @port.setter
 def port(self, port):
 if port[:3] != "COM":
 return
 self.__port = port
 @property
 def identifier(self):
 return self.__identifier
 @identifier.setter
 def identifier(self, letter):
 if type(letter) is not str:
 return
 if letter[-1] != ':':
 letter += ':'
 self.__identifier = letter
 def serialCommand(self, command):
 """
 Wrapper for ArduinoI2C serialCommand function
 """
 return self.__i2c.serialCommand(command)
 def close(self):
 self.__i2c.close()
 def connectPort(self):
 self.__i2c.connectPort()

This implementation feels a little dumb to me. Is there a more elegant way to implement something like this?

Here is some further clarification on the functionality of the class:

self.__i2c is an instance of an Arduino I2C communication module that was internally developed at my company. It allows users to send commands to I2C devices over the Arduino's I2C bus. setPort and connectPort are setting and connect a COM port for the specific Arduino.

self.__id is a serial number generated for the Arduino from a company-MySQL database. It is set elsewhere in the program.

self.__switch_version and self.__i2c_version are two floats stored on the Arduino that are the version number of some software running on the Arduino.

self.__tree_item is a wx.TreeItemId (I believe it's stored as just an int) that is the id of the wx.TreeItem stored in a wx.TreeCtrl elsewhere in the program.

self.__address_list is a list of ints which are the I2C addresses of devices discovered on the Arduino's I2C bus.

self.__port is a string which holds the COM port the Arduino is inserted in.

self.__identifier is a letter which identifies the Arduino elsewhere in the program. The program expects the user has multiple Arduinos concurrently being used, so a letter is a quick shorthand to easily identify them.

Answer 1

Here is some feedback I have. I haven't had a chance to work with Arduino much, but there are some things that stand out to me:

 # Nothing to glaring in your constructor, but some tidbits:
 # I might add some defaults. 
 # I also noticed you have a lot of items in your constructor. 
 # Maybe **kwargs is warranted.
def __init__(self, tree_item, port=8080, identifier):
or
def __init__(self, **kwargs)
 # the number of arguments is borderline imho. I usually find if I
 # have too large a number (subjective), there is opportunity to 
 # break out the code into more classes. 
 self.__i2c = ArduinoI2C()
 self.__i2c.setPort(port)
 self.__i2c.connectPort()
 self.__id = None
 self.__i2c_version = None
 self.__switch_version = None
 self.__tree_item = tree_item
 self.__address_list = []
 self.__port = port
 self.__identifier = identifier
# Typically methods indicate some sort of action. i.e. get_id, show_id.
# specifying a noun is usually grounds for a class/instance variable.
# I would consider renaming your method to "def get_id(self):"
@property
def id(self):
 if self.__id is None:
 try:
 self.__id = self.__i2c.getID()
 except IOError: 
 # It may be good to add some logging. 
 # python logging module is great.
 self.__id = "NO CONNECTION"
 return self.__id
# ditto method name
@property
def i2c_version(self):
 if self.__i2c_version is None:
 try:
 self.__i2c_version = self.__i2c.i2cVersion()
 except IOError:
 # ditton on the logging.
 self.__i2c_version = "NO CONNECTION"
 return self.__i2c_version
# ditto method name
@property
def switch_version(self):
 if self.__switch_version is None:
 try:
 self.__switch_version = self.__i2c.switchVersion()
 except IOError:
 self.__switch_version = "NO CONNECTION"
 return self.__switch_version
# ditto method name
@property
def tree_item(self):
 return self.__tree_item
# ditto method name
@tree_item.setter
def tree_item(self, item):
 if type(item) is not wx.TreeItemId:
 return
 self.__tree_item = item
# ditto method name
@property
def address_list(self):
 return self.__address_list
# ditto method name
@address_list.setter
def address_list(self, addr_list):
 if type(addr_list) is not list:
 return
 self.__address_list = addr_list
# ditto method name
@property
def port(self):
 return self.__port
# ditto method name
@port.setter
def port(self, port):
 if port[:3] != "COM":
 return
 self.__port = port
# ditto method name
@property
def identifier(self):
 return self.__identifier
# ditto method name
@identifier.setter
def identifier(self, letter):
 if type(letter) is not str:
 return
 if letter[-1] != ':':
 letter += ':'
 self.__identifier = letter
# ditto, and its not idiomatic to use camelCase in a method name.
# def run_serial_command(self, command)
def serialCommand(self, command):
 """
 Wrapper for ArduinoI2C serialCommand function
 """
 return self.__i2c.serialCommand(command)
# this method name is ok
def close(self):
 self.__i2c.close()
# this one two.. ditch the camelCase for snake_case connect_port
def connectPort(self):
 self.__i2c.connectPort()

It looks like there might be some opportunity for grouping your commands into separate classes, however not sure if you have enough per group yet.

i.e.

class ArduinoVersions(ARCArduino):
# this might be to ambiguous, but just trying to give the general idea.
class ArduinoProperties(ArcaArduino): 

I have a feeling your command set will grow, so breaking this out may provide better organization in the future.

• sawyermclane: For the functions where you say change them to "get_id" or something along those lines, I thought that when creating an attribute with the @property decorator, the method name had to be the name of the intended attribute. Also for the serialCommand and connectPort methods, they're meant to shadow existing commands in the ArduinoI2C library, so an ARCArduino object can be used similarly to an ArduinoI2C object.
• fr00z1: Sorry for the late reply, haven't logged in for a while. You are 100% correct on @property. Miss on my part. I blame lack of sleep.. :-) Your reasons on the other methods may be valid as well, however the potential confusion (imho) is the method names will need a double take as camelCase is a convention typically reserved for class names (or if following an existing convention). If I were looking at your code for the 1st time, I most likely would miss the arduino lib correlation. pep8 (See class/method/function conventions)