Skip to content

Commit

Permalink
refactor Signal to attr
Browse files Browse the repository at this point in the history
  • Loading branch information
@ebroecker
ebroecker committed Jun 29, 2018
1 parent dd34a77 commit a889352
Showing 1 changed file with 61 additions and 69 deletions.
130 changes: 61 additions & 69 deletions canmatrix/canmatrix.py
View file
Expand Up @@ -29,6 +29,7 @@

from __future__ import division
import math
import attr

from collections import OrderedDict

Expand Down Expand Up @@ -185,7 +186,7 @@ def remove(self, BU):
"""
remove Boardunit/ECU to list
"""
if BU.name.strip() not in self._list:
if BU.name.strip() in self._list:

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 1, 2018

Collaborator

Are you mixing random bug fixes in here? Probably better to keep this a pure attrs-conversion branch.

This comment has been minimized.

Sign in to view

Copy link
@ebroecker

ebroecker Jul 1, 2018

Author Owner

you are right. this was a mistake, this should go to the delvelopement-branch...
self._list.remove(BU)

def byName(self, name):
Expand Down Expand Up @@ -221,6 +222,7 @@ def normalizeValueTable(table):
return {int(k): v for k, v in table.items()}


@attr.s
class Signal(object):
"""
contains on Signal of canmatrix-object
Expand All @@ -234,71 +236,50 @@ class Signal(object):
_multiplex ('Multiplexor' or Number of Multiplex)
"""

def __init__(self, name, **kwargs):
self.mux_val = None
self.is_multiplexer = False

def multiplex(value):
if value is not None and value != 'Multiplexor':
multiplex = int(value)
self.mux_val = int(value)
else:
self.is_multiplexer = True
multiplex = value
return multiplex

float_factory = kwargs.pop('float_factory', float)

args = [
('startBit', 'startbit', int, 0),
('signalSize', 'signalsize', int, 0),
('is_little_endian', 'is_little_endian', bool, True),
('is_signed', 'is_signed', bool, True),
('factor', 'factor', float_factory, 1),
('offset', 'offset', float_factory, 0),
('min', 'min', float_factory, None),
('max', 'max', float_factory, None),
('unit', 'unit', None, ""),
('receiver', 'receiver', None, []),
('comment', 'comment', None, None),
('multiplex', 'multiplex', multiplex, None),
('mux_value', 'mux_value', None, None),
('is_float', 'is_float', bool, False),
('enumeration', 'enumeration', str, None),
('comments', 'comments', None, {}),
('attributes', 'attributes', None, {}),
('values', '_values', None, {}),
('calc_min_for_none', 'calc_min_for_none', bool, True),
('calc_max_for_none', 'calc_max_for_none', bool, True),
('muxValMax', 'muxValMax', int, 0),
('muxValMin', 'muxValMin', int, 0),
('muxerForSignal', 'muxerForSignal', str, None)
]

for arg_name, destination, function, default in args:
try:
value = kwargs[arg_name]
except KeyError:
value = default
else:
kwargs.pop(arg_name)
if function is not None and value is not None:
value = function(value)
setattr(self, destination, value)

if len(kwargs) > 0:
raise TypeError('{}() got unexpected argument{} {}'.format(
self.__class__.__name__,
's' if len(kwargs) > 1 else '',
', '.join(kwargs.keys())
))


# in case missing min/max calculation is enabled, be sure it happens
self.setMin(self.min)
self.setMax(self.max)

self.name = name
name = attr.ib(default = "")
float_factory = attr.ib(default=float)
startBit = attr.ib(type=int, default=0)
signalSize = attr.ib(type=int, default = 0)
is_little_endian = attr.ib(type=bool, default = True)
is_signed = attr.ib(type=bool, default = True)
# float_factory = attr.ib(default = float)
float_factory = float

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 1, 2018

Collaborator

Drop this... it's a proper attrs attribute above already.
offset = attr.ib(convert = float_factory, default = 0.0)

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 1, 2018

Collaborator

converter is the modern name for convert.

https://attrs.readthedocs.io/en/stable/api.html
Deprecated since version 17.4.0: convert

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 1, 2018

Collaborator

Oh boy, float_factory... First, float_factory is itself a parameter so we can't use it like this.
At the point it's being evaluated here it's a attr._make._CountingAttr, the thing that the @attr.s decorator looks for. Second, converter doesn't get applied to the provided default so that's broken as well.

The default can be addressed by defining this attribute after float_factory (as it already is) then defining the default as:

@offset.default
def _(self):
    return self.float_factory(0)

I'll have to think about the converter since the thing that gets called doesn't get passed anything except the only gets the value, not self. "It is given the passed-in value, and the returned value will be used as the new value of the attribute." Maybe I could add @attribute.converter for them and have it also include self. Maybe.

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 1, 2018

Collaborator

python-attrs/attrs#240

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 2, 2018

Collaborator

Alrighty, got a first pass at a converter decorator with a PR uploaded (python-attrs/attrs#404). It's definitely a work in progress needing at least doc updates even if the code passes muster.
factor = attr.ib(convert = float_factory, default = 0.0)

# offset = attr.ib(converter = float_factory, default = 0.0)

min = attr.ib(convert=float_factory)
@min.default
def setDefaultMin(self):
return self.calcMin()

max = attr.ib(convert = float_factory)
@max.default
def setDefaultMax(self):
return self.calcMax()

unit = attr.ib(type=str, default ="")
receiver = attr.ib(default =[])
comment = attr.ib(default = None)
_multiplex = attr.ib(default =None)

mux_value = attr.ib(default = None)
is_float = attr.ib(type=bool, default = False)
enumeration = attr.ib(type=str, default = None),
comments = attr.ib(type=dict, default ={})
attributes = attr.ib(type=dict, default ={})
values = attr.ib(type=dict, default ={})

This comment has been minimized.

Sign in to view

Copy link
@altendky

altendky Jul 1, 2018

Collaborator

Are you looking to do type annotations? I don't think type= does anything else. I personally haven't bothered trying to get into type annotations yet. MIght be worth reading up on that separately if you want it. For example, this should probably be typing.MutableMapping instead of dict so duck typing can be retained without warnings for people that use tools to check the typing info.

https://docs.python.org/3/library/typing.html#typing.MutableMapping
calc_min_for_none = attr.ib(type=bool, default = True)
calc_max_for_none = attr.ib(type=bool, default = True)
muxValMax = attr.ib(default = 0)
muxValMin = attr.ib(default = 0)
muxerForSignal= attr.ib(type=str, default = None)

def __attrs_post_init__(self):
self.multiplex(self._multiplex)

@property
def values(self):
Expand All @@ -315,6 +296,17 @@ def spn(self):
def values(self, valueTable):
self._values = normalizeValueTable(valueTable)

def multiplex(self, value):
self.mux_val = None
self.is_multiplexer = False
if value is not None and value != 'Multiplexor':
ret_multiplex = int(value)
self.mux_val = int(value)
else:
self.is_multiplexer = True
ret_multiplex = value
return ret_multiplex


def attribute(self, db, attributeName):
if attributeName in self.attributes:
Expand Down Expand Up @@ -390,18 +382,18 @@ def setStartbit(self, startBit, bitNumbering=None, startLittle=None):
self.startbit = startBit

def getStartbit(self, bitNumbering=None, startLittle=None):
startBit = self.startbit
startBit = self.startBit
# convert from big endian start bit at
# start bit(msbit) to end bit(lsbit)
if startLittle is True and self.is_little_endian is False:
startBit = startBit + self.signalsize - 1
startBit = startBit + self.signalSize - 1
# bit numbering not consistent with byte order. reverse
if bitNumbering is not None and bitNumbering != self.is_little_endian:
startBit = startBit - (startBit % 8) + 7 - (startBit % 8)
return int(startBit)

def calculateRawRange(self):
rawRange = 2 ** self.signalsize
rawRange = 2 ** self.signalSize
if self.is_signed:
rawRange /= 2
return (-rawRange if self.is_signed else 0,
Expand Down Expand Up @@ -1187,8 +1179,8 @@ def recalcDLC(self, strategy):
if "force" == strategy:
maxBit = 0
for sig in frame.signals:
if sig.getStartbit() + int(sig.signalsize) > maxBit:
maxBit = sig.getStartbit() + int(sig.signalsize)
if sig.getStartbit() + int(sig.signalSize) > maxBit:
maxBit = sig.getStartbit() + int(sig.signalSize)
frame.size = math.ceil(maxBit / 8)

def renameEcu(self, old, newName):
Expand Down

1 comment on commit a889352

@altendky
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a nitpick, the library is referred to as attrs in text, not attr... even if you do import attr. :|

Please sign in to comment.