A clean way to program an interface

[rh0dium]

Hi all,

I got this new radio scanner (toy!!) this weekend and I can access it
via a serial cable. I want to write a interface around it but I am
looking for some suggestions. I thought at first I would simply class
the Scanner and write the various methods as attibutes similar to
below.. But then I thought about it and I don't like it for a couple
of obvious reasons - there isn't any data checking, and it's terribly
repetitive.

So I started searching on google - and I think it might be better to
use a (sub)class for each function ( class STS, PRG, etc.). where the
attributes are accessible as both dict items and lists - which
ultimately go and "set" the scanner.

I guess my question is this.

I have a function SIN which when one argument is given does an
effective "get" from the scanner - this returns some 10+ values. When
some 10+ parameters are given it does an effective "set" to the
scanner. Given that I can't remember all 10+ parameters I attempted
below to represent them as a dict where the key tells me what the heck
the parameter is supposed to represent. So a simple translation
occurs on both the set and get which splits the list into a named dict
and vice versa. Question 1 - I think I should be checking the data
before a set - is this a smart thing or should I just do a try/
except? Question 2 - Since I only want to do this once (ok twice)
should I represent each function as a class or should I keep the code
I have below?

Thanks



== My garbage.. ==

class Scanner():

def __init__(self, *args, **kwargs):

self.uniden=UnidenConnection.UnidenConnection(port=kwargs.get('port',
PORT), bps=kwargs.get('bps', 115200), loglevel=self.loglevel)

def STS(self):
return self.uniden.write("STS")

def PRG(self):
"""Go into programming mode"""
if self.uniden.write(sys._getframe().f_code.co_name)[0]=="OK":
return True
else: return False

def PRGWrite(self,command):
data = self.uniden.write(command)
if data[0] == "NG":
if self.PRG():
data = self.uniden.write(command)
if not self.EPG():
raise IOError, "Unable to exit programming
mode"
if data[0] == "ERR":
raise IOError, "Error when running command %s" % command
return data

def EPG(self):
"""Get out of programming mode"""
if self.uniden.write(sys._getframe().f_code.co_name)[0]=="OK":
return True
else: return False

def SCT(self):
"""Get the system count"""
self.systems =
int(self.PRGWrite(sys._getframe().f_code.co_name)[0])
return self.systems

def SIH(self):
"""Get the site index head"""
self.sih = int(self.PRGWrite(sys._getframe().f_code.co_name)
[0])
return self.sih

def SIT(self):
"""Get the site index tail"""
self.sit = int(self.PRGWrite(sys._getframe().f_code.co_name)
[0])
return self.sit

def FWD(self,idx):
"""Get the next site index"""
fwd = int(self.PRGWrite("%s,%s" %
(sys._getframe().f_code.co_name,idx))[0])
return fwd

def SIF(self, idx, args=None):
"""Get / Set Site Info"""
if args is None:
p=self.PRGWrite("%s,%s" %
(sys._getframe().f_code.co_name,idx))
t={}

t["site_type"],t["name"],t["quick_key"],t["hld"],t["lout"],t["mod"], \
t["att"],t["c-
ch"],t["apco"],t["threshold"],t["rev_index"],t["fwd_index"], \

t["sys_index"],t["chn_head"],t["chn_tail"],t["seq_no"],t["start_key"],
\

t["latitude"],t["longitude"],t["range"],t["gps_enable"],t["rsv"]=p
delitems=[]
for item in t:
if t[item]=="": delitems.append(item)
for x in delitems: del t[x]
if len(t.keys())==0:
raise IOError, "Unable to program %s - Returned
Nothing" % (sys._getframe().f_code.co_name)
else:
return t
else:
args["rsv"]=""

s=[ args["index"],args["name"],args["quick_key"],args["hld"],args["lout"],args["mod"],
\
args["att"],args["c-
ch"],args["apco"],args["threshold"],args["start_key"], \

args["latitude"],args["longitude"],args["range"],args["gps_enable"],args["rsv"]]
p=self.PRGWrite("%s,%s" %
(sys._getframe().f_code.co_name,",".join(s)))
if p[0] != "OK":
raise IOError, "Unable to program %s - Returned %s" %
(sys._getframe().f_code.co_name,p[0])
else: return True

def SIN(self, idx, args=None):
"""Get / Set System Info"""
if args is None:
p=self.PRGWrite("%s,%s" %
(sys._getframe().f_code.co_name,idx))
t={}
t["sys_type"],t["name"],t["quick_key"],t["hld"],t["lout"],
\

t["dly"],t["skp"],t["rsv"],t["rsv"],t["apco"],t["threshold"], \

t["rev_index"],t["fwd_index"],t["chn_grp_head"],t["chn_grp_tail"], \

t["seq_no"],t["start_key"],t["record"],t["latitude"],t["longitude"], \
t["range"],t["gps enable"],t["rsv"]=p
delitems=[]
for item in t:
if t[item]=="": delitems.append(item)
for x in delitems: del t[x]
if len(t.keys())==0:
raise IOError, "Unable to program %s - Returned
Nothing" % (sys._getframe().f_code.co_name)
else:
return t
else:
args["rsv"]=""

s=[ args["index"],args["name"],args["quick_key"],args["hld"],args["lout"],args["dly"],
\

args["skp"],args["rsv"],args["rsv"],args["apco"],args["threshold"],args["start_key"],
\

args["record"],args["latitude"],args["longitude"],args["range"],args["gps_enable"],args["rsv"]]
p=self.PRGWrite("%s,%s" %
(sys._getframe().f_code.co_name,",".join(s)))
if p[0] != "OK":
raise IOError, "Unable to program %s - Returned %s" %
(sys._getframe().f_code.co_name,p[0])
else: return True

 

[Diez B. Roggisch]

Hi all,

I got this new radio scanner (toy!!) this weekend and I can access it
via a serial cable. I want to write a interface around it but I am
looking for some suggestions. I thought at first I would simply class
the Scanner and write the various methods as attibutes similar to
below.. But then I thought about it and I don't like it for a couple
of obvious reasons - there isn't any data checking, and it's terribly
repetitive.

So I started searching on google - and I think it might be better to
use a (sub)class for each function ( class STS, PRG, etc.). where the
attributes are accessible as both dict items and lists - which
ultimately go and "set" the scanner.

I guess my question is this.

I have a function SIN which when one argument is given does an
effective "get" from the scanner - this returns some 10+ values. When
some 10+ parameters are given it does an effective "set" to the
scanner. Given that I can't remember all 10+ parameters I attempted
below to represent them as a dict where the key tells me what the heck
the parameter is supposed to represent. So a simple translation
occurs on both the set and get which splits the list into a named dict
and vice versa. Question 1 - I think I should be checking the data
before a set - is this a smart thing or should I just do a try/
except? Question 2 - Since I only want to do this once (ok twice)
should I represent each function as a class or should I keep the code
I have below?

THROW IT AWAY!!!!

Seriously. That's one of the most convoluted, incomprehensible pieces of
python I've seen. Ever.

All the sys._getframe()-stuff has to go. Really. There are about a dozen
pieces of code worldwide that are allowed to have that in them. Maybe two
dozen.

And while I'm having difficulties grasping what you actually mean by " I
represent each function as a class or should I keep the code
I have below?", I've done my fair share of serial programming. And I would
say that having one class that exposes different methods for commands is
the way to go.

But first - throw that code away. Fast. Faster.

Diez

 

[rh0dium]

THROW IT AWAY!!!!

Seriously. That's one of the most convoluted, incomprehensible pieces of
python I've seen. Ever.

All the sys._getframe()-stuff has to go. Really. There are about a dozen
pieces of code worldwide that are allowed to have that in them. Maybe two
dozen.

And while I'm having difficulties grasping what you actually mean by " I
represent each function as a class or should I keep the code
I have below?", I've done my fair share of serial programming. And I would
say that having one class that exposes different methods for commands is
the way to go.

But first - throw that code away. Fast. Faster.

Diez

Hi Diez,

Totally understand your confusion with sys._getframe().f_code.co_name
- that was borne out of the fact that I was creating a method for each
serial function and it was repeating.. over and over.. So I got tired
of changing the three items that differentiated each method so I
simplified it a bit. Perhaps too much. So can I ask you to show or
point me to a good interface example? I think we both can agree that
fundamentally this is pretty simple but I want to do it right and an
solid class example of interface programming would be nice.

Thanks

rh0dium

 

[Diez B. Roggisch]

Hi Diez,

Totally understand your confusion with sys._getframe().f_code.co_name
- that was borne out of the fact that I was creating a method for each
serial function and it was repeating.. over and over..
So I got tired
of changing the three items that differentiated each method so I
simplified it a bit. Perhaps too much.

I can't say that it has become simpler... It would have been if you at least
created a function for it. But even then, it's certainly the wrong way to
go.

So can I ask you to show or
point me to a good interface example? I think we both can agree that
fundamentally this is pretty simple but I want to do it right and an
solid class example of interface programming would be nice.

I don't understand what you mean by "interface programming". If you tell us
how the serial protocol works, and what it accomplishes, one might come up
with a clean solution.

But that depends on the actual protocol.

Diez

 

[Paul McGuire]

I believe the OP is talking about "interface" as in "hardware
interface", using some form of serial communication. His example does
not use pyserial, but it does refer to a "UnidenConnection" class,
with parameters such as bitrate, port, etc. for what looks like serial
communication.

Some general comments:

1. Much of the OP's use of sys._getframe().f_code.co_name coincides
with his naming of certain methods with the same name as the command
to be sent down the serial line. "STS", "EPG", "PRG", "SIN", etc. are
all 3-character command codes that get sent down the wire, along with
the appropriate values for the particular command. While many of
these routines have *some* code in common, only a few have their
entire method body duplicated with one or more other methods.
Refactoring a couple of these into code closures, you can get:

@staticmethod
def makeStatusCheckCommandFunction(cmd):
def fn(self):
return ( self.uniden.write(cmd)[0]=="OK" )
return fn
PRG = makeStatusCheckCommandFunction("PRG")
EPG = makeStatusCheckCommandFunction("EPG")


@staticmethod
def makeCmdFunction(cmd,attrname):
def fn(self):
setattr(self, attrname, int(self.PRGWrite(cmd)[0]))
return getattr(self,attrname)
return fn
SCT = makeCmdFunction("SCT","systems")
SIH = makeCmdFunction("SIH","sih")
SIT = makeCmdFunction("SIT","sit")

Now there is no sys._getframe().f_code.co_name legerdemain going on,
the command strings are just passed along as the argument cmd, and the
functions themselves are named the same for some level of self-
documentation and/or convenience.

For the remaining methods, I declare a local variable named cmd and
assign to it the write mnemonic. Yes, I know it violates DRY ("Don't
Repeat Yourself"), but it avoids the getframe() stuff and again, is at
least a snippet of self-documentation.

2. This code for reading and unpacking all the non-blank values feels
ugly:

cmd = "SIF"
if args is None:
p=self.PRGWrite("%s,%s" % (cmd,idx))
t={}
t["site_type"],t["name"],t["quick_key"],t["hld"],t["lout"],\
t["mod"],t["att"],t["cch"],t["apco"],t["threshold"],\
t["rev_index"],t["fwd_index"],t["sys_index"],t["chn_head"],
\
t["chn_tail"],t["seq_no"],t["start_key"],t["latitude"],\
t["longitude"],t["range"],t["gps_enable"],t["rsv"]=p

delitems=[]
for item in t:
if t[item]=="": delitems.append(item)
for x in delitems: del t[x]

How about this instead?:

cmd = "SIF"
if args is None:
p=self.PRGWrite("%s,%s" % (cmd,idx))
names = "site_type name quick_key hld lout mod att cch "\
"apco threshold rev_index fwd_index sys_index "\
"chn_head chn_tail seq_no start_key latitude " \
"longitude range gps_enable rsv".split()

t = dict( k,v for k,v in zip(names,p) if v != "" )


HTH,
-- Paul

 

[rh0dium]

I believe the OP is talking about "interface" as in "hardware
interface", using some form of serial communication. His example does
not use pyserial, but it does refer to a "UnidenConnection" class,
with parameters such as bitrate, port, etc. for what looks like serial
communication.

Yes - You are correct UnidenConnection is nothing more than a pyserial
connection which handles all of the setup and returns (converting
strings to list objects etc).

Some general comments:

1. Much of the OP's use of sys._getframe().f_code.co_name coincides
with his naming of certain methods with the same name as the command
to be sent down the serial line. "STS", "EPG", "PRG", "SIN", etc. are
all 3-character command codes that get sent down the wire, along with
the appropriate values for the particular command. While many of
these routines have *some* code in common, only a few have their
entire method body duplicated with one or more other methods.
Refactoring a couple of these into code closures, you can get:

@staticmethod
def makeStatusCheckCommandFunction(cmd):
def fn(self):
return ( self.uniden.write(cmd)[0]=="OK" )
return fn
PRG = makeStatusCheckCommandFunction("PRG")
EPG = makeStatusCheckCommandFunction("EPG")

@staticmethod
def makeCmdFunction(cmd,attrname):
def fn(self):
setattr(self, attrname, int(self.PRGWrite(cmd)[0]))
return getattr(self,attrname)
return fn
SCT = makeCmdFunction("SCT","systems")
SIH = makeCmdFunction("SIH","sih")
SIT = makeCmdFunction("SIT","sit")

Hmm very interesting - I've never played with code closures. Very
cool - I need to learn me about these thingy's

Now there is no sys._getframe().f_code.co_name legerdemain going on,
the command strings are just passed along as the argument cmd, and the
functions themselves are named the same for some level of self-
documentation and/or convenience.

For the remaining methods, I declare a local variable named cmd and
assign to it the write mnemonic. Yes, I know it violates DRY ("Don't
Repeat Yourself"), but it avoids the getframe() stuff and again, is at
least a snippet of self-documentation.

2. This code for reading and unpacking all the non-blank values feels
ugly:

cmd = "SIF"
if args is None:
p=self.PRGWrite("%s,%s" % (cmd,idx))
t={}
t["site_type"],t["name"],t["quick_key"],t["hld"],t["lout"],\
t["mod"],t["att"],t["cch"],t["apco"],t["threshold"],\
t["rev_index"],t["fwd_index"],t["sys_index"],t["chn_head"],
\
t["chn_tail"],t["seq_no"],t["start_key"],t["latitude"],\
t["longitude"],t["range"],t["gps_enable"],t["rsv"]=p

delitems=[]
for item in t:
if t[item]=="": delitems.append(item)
for x in delitems: del t[x]

How about this instead?:

cmd = "SIF"
if args is None:
p=self.PRGWrite("%s,%s" % (cmd,idx))
names = "site_type name quick_key hld lout mod att cch "\
"apco threshold rev_index fwd_index sys_index "\
"chn_head chn_tail seq_no start_key latitude " \
"longitude range gps_enable rsv".split()

t = dict( k,v for k,v in zip(names,p) if v != "" )

This is absolutely the right way to do this - I totally like this -
Super!!

HTH,
-- Paul

Hey thanks a lot!!

 

[rh0dium]

Hi all,

OK so I've started re-writing this based on the feedback you all gave
me. How does this look?

class Scanner:

def __init__(self, *args, **kwargs):
"""description"""
# Setup Logging
self.log = logging.getLogger(module+self.__class__.__name__)
self.loglevel = kwargs.get('loglevel', logging.WARN)
self.setLoglevel(self.loglevel)

self.serialport=kwargs.get('port', PORT)
self.serialbps=kwargs.get('bps', 115200)

self.uniden=UnidenConnection.UnidenConnection(port=self.serialport,
bps=self.serialbps, loglevel=self.loglevel)

if self.inProgramMode():
self.PRGMode=True
else: self.PRGMode=False


def setLoglevel(self,loglevel):
"""setLog log level"""
if loglevel is not False:
self.log.setLevel(loglevel)
self.log.debug("Setting Logging level to %s" % loglevel)
else:
self.log.setLevel(logging.WARN)

if self.loglevel == logging.DEBUG: self.debug = True
else: self.debug=False

def ProgramMode(fn):
def _prgmode(self,*args,**kwargs):
progmode=True
if not self.PRGMode:
self.log.debug("Going into programming mode")
progmode=False
self.PRG()
ret = fn(self,*args,**kwargs)
if not progmode:
self.log.debug("Getting out of programming mode")
self.EPG()
return ret
return _prgmode

def __orderedDictValues(self,orderedlist,dictvals):

rlist = []
for x in orderedlist:
if dictvals.has_key(x) or x == "rsv":
if x == "rsv": rlist.append("")
else: rlist.append(str(dictvals[x]))
else:
break
return rlist

def STS(self):
"""Get Current Status"""
cmd = "STS"
p = self.uniden.write(cmd)
retargs = "dsp_form l1_char l1_mode l2_char l2_mode l3_char
l3_mode \
l4_char l4_mode l5_char l5_mode l6_char l6_mode
l7_char \
l7_mode l8_char l8_mode sql mut rsv wat led_1 led_2
sig_lvl \
bk_color bk_dimmer".split()

t = dict((k,v) for k, v in zip(retargs,p) if v != "")
return t

def PRG(self):
"""Enter Programming Mode"""
self.PRGMode=True
return self.uniden.write("PRG")

def EPG(self):
"""Exit Programming Mode"""
self.PRGMode=True
return self.uniden.write("EPG")

@ProgramMode
def SIN(self, *args, **kwargs):
"""Get/Set System Index"""

cmd = "SIN"
iptargs="index name quick_key hld lout dly skp rsv rsv apco \
threshold start_key record latitude longitude range \
gps_enable rsv".split()

iptarglist=self.__orderedDictValues(iptargs,kwargs)

if len(iptarglist) != len(iptargs) and len(iptarglist) != 1:
raise SyntaxError, "Missing arguments for %s expected 1 or
%d got %d - given %s" % (cmd, len(iptargs), len(iptarglist), kwargs)

p = self.uniden.write("%s,%s" % (cmd,",".join(iptarglist)))

retargs = "sys_type name quick_key hld lout dly skp rsv rsv
apco \
threshold rev_index fwd_index chn_grp_head
chn_grp_tail \
seq_no start_key record latitude longitude range
gps enable \
rsv".split()

t = dict((k,v) for k, v in zip(retargs,p) if v != "")

return t


def inProgramMode(self):
try:
print"PRGMODE",self.STS()
if self.STS()["l2_char"].strip()=="Remote Mode":
print "IN PRG"
return True
else: return False
except: return False