From 996dff22316405746230442ed2abfb8d1de0b9be Mon Sep 17 00:00:00 2001 From: bashwork Date: Mon, 1 Apr 2013 21:37:02 -0700 Subject: [PATCH] Reworking the transaction managers to be explicit - Serial framers use the FIFO manager (results in order) - Socket framers use the Dict manager (tid -> result) - Fixed tests and removed bad global managers - Managers no longer use global state (now instance) --- doc/sphinx/library/transaction.rst | 6 ++ pymodbus/client/async.py | 24 +++-- pymodbus/client/sync.py | 9 +- pymodbus/transaction.py | 150 ++++++++++++++++++++++------- test/test_transaction.py | 65 ++++++++++--- 5 files changed, 189 insertions(+), 65 deletions(-) diff --git a/doc/sphinx/library/transaction.rst b/doc/sphinx/library/transaction.rst index 0d1a05876..b6ee89a8a 100644 --- a/doc/sphinx/library/transaction.rst +++ b/doc/sphinx/library/transaction.rst @@ -12,6 +12,12 @@ API Documentation .. automodule:: pymodbus.transaction +.. autoclass:: DictTransactionManager + :members: + +.. autoclass:: FifoTransactionManager + :members: + .. autoclass:: ModbusSocketFramer :members: diff --git a/pymodbus/client/async.py b/pymodbus/client/async.py index d8e7b4064..80c755965 100644 --- a/pymodbus/client/async.py +++ b/pymodbus/client/async.py @@ -35,7 +35,9 @@ def process(): from twisted.internet import defer, protocol from pymodbus.factory import ClientDecoder from pymodbus.exceptions import ConnectionException -from pymodbus.transaction import ModbusSocketFramer, ModbusTransactionManager +from pymodbus.transaction import ModbusSocketFramer +from pymodbus.transaction import FifoTransactionManager +from pymodbus.transaction import DictTransactionManager from pymodbus.client.common import ModbusClientMixin from twisted.python.failure import Failure @@ -46,12 +48,6 @@ def process(): _logger = logging.getLogger(__name__) -#---------------------------------------------------------------------------# -# A manager for the transaction identifiers -#---------------------------------------------------------------------------# -_manager = ModbusTransactionManager() - - #---------------------------------------------------------------------------# # Connected Client Protocols #---------------------------------------------------------------------------# @@ -66,10 +62,11 @@ def __init__(self, framer=None): :param framer: The framer to use for the protocol ''' - self.framer = framer or ModbusSocketFramer(ClientDecoder()) - serial = not isinstance(framer, ModbusSocketFramer) - self.transaction = ModbusTransactionManager(self, serial) self._connected = False + self.framer = framer or ModbusSocketFramer(ClientDecoder()) + if isinstance(framer, ModbusSocketFramer): + self.transaction = DictTransactionManager(self) + else: self.transaction = FifoTransactionManager(self) def connectionMade(self): ''' Called upon a successful client connection. @@ -99,7 +96,7 @@ def execute(self, request): ''' Starts the producer to send the next request to consumer.write(Frame(request)) ''' - request.transaction_id = _manager.getNextTID() + request.transaction_id = self.transaction.getNextTID() packet = self.framer.buildPacket(request) self.transport.write(packet) return self._buildResponse(request.transaction_id) @@ -155,8 +152,9 @@ def __init__(self, framer=None): :param framer: The framer to use for the protocol ''' self.framer = framer or ModbusSocketFramer(ClientDecoder()) - serial = not isinstance(framer, ModbusSocketFramer) - self.transaction = ModbusTransactionManager(self, serial) + if isinstance(framer, ModbusSocketFramer): + self.transaction = DictTransactionManager(self) + else: self.transaction = FifoTransactionManager(self) def datagramReceived(self, data, (host, port)): ''' Get response, check for valid message, decode result diff --git a/pymodbus/client/sync.py b/pymodbus/client/sync.py index 89e9bd48d..061fe0d00 100644 --- a/pymodbus/client/sync.py +++ b/pymodbus/client/sync.py @@ -5,7 +5,8 @@ from pymodbus.factory import ClientDecoder from pymodbus.exceptions import NotImplementedException, ParameterException from pymodbus.exceptions import ConnectionException -from pymodbus.transaction import ModbusTransactionManager +from pymodbus.transaction import FifoTransactionManager +from pymodbus.transaction import DictTransactionManager from pymodbus.transaction import ModbusSocketFramer, ModbusBinaryFramer from pymodbus.transaction import ModbusAsciiFramer, ModbusRtuFramer from pymodbus.client.common import ModbusClientMixin @@ -33,9 +34,10 @@ def __init__(self, framer): :param framer: The modbus framer implementation to use ''' - serial = not isinstance(framer, ModbusSocketFramer) self.framer = framer - self.transaction = ModbusTransactionManager(self, serial) + if isinstance(framer, ModbusSocketFramer): + self.transaction = DictTransactionManager(self) + else: self.transaction = FifoTransactionManager(self) #-----------------------------------------------------------------------# # Client interface @@ -133,7 +135,6 @@ def connect(self): if self.socket: return True try: self.socket = socket.create_connection((self.host, self.port), Defaults.Timeout) - self.transaction = ModbusTransactionManager(self) except socket.error, msg: _logger.error('Connection to (%s, %s) failed: %s' % \ (self.host, self.port, msg)) diff --git a/pymodbus/transaction.py b/pymodbus/transaction.py index 0633b22da..70f5b1ba1 100644 --- a/pymodbus/transaction.py +++ b/pymodbus/transaction.py @@ -38,24 +38,13 @@ class ModbusTransactionManager(object): This module helps to abstract this away from the framer and protocol. ''' - __tid = Defaults.TransactionId - __transactions = {} - - def __init__(self, client=None, fifo=False): + def __init__(self, client): ''' Initializes an instance of the ModbusTransactionManager :param client: The client socket wrapper - :param fifo: Should this just return results in FIFO order ''' + self.tid = Defaults.TransactionId self.client = client - self.fifo = fifo - - def __iter__(self): - ''' Iterater over the current managed transactions - - :returns: An iterator of the managed transactions - ''' - return iter(self.__transactions.keys()) def execute(self, request): ''' Starts the producer to send the next request to @@ -88,12 +77,9 @@ def addTransaction(self, request, tid=None): After being sent, the request is removed. :param request: The request to hold on to - :param tid: The transaction id to attach this request with + :param tid: The overloaded transaction id to use ''' - if tid == None: - tid = request.transaction_id - _logger.debug("Adding transaction %d" % tid) - ModbusTransactionManager.__transactions[tid] = request + raise NotImplementedException("addTransaction") def getTransaction(self, tid): ''' Returns a transaction matching the referenced tid @@ -102,21 +88,14 @@ def getTransaction(self, tid): :param tid: The transaction to retrieve ''' - if self.fifo: - if len(ModbusTransactionManager.__transactions): - return ModbusTransactionManager.__transactions.popitem()[1] - else: return None - return ModbusTransactionManager.__transactions.pop(tid, None) + raise NotImplementedException("getTransaction") def delTransaction(self, tid): ''' Removes a transaction matching the referenced tid :param tid: The transaction to remove ''' - if self.fifo: - if len(ModbusTransactionManager.__transactions): - ModbusTransactionManager.__transactions.popitem() - ModbusTransactionManager.__transactions.pop(tid, None) + raise NotImplementedException("delTransaction") def getNextTID(self): ''' Retrieve the next unique transaction identifier @@ -126,13 +105,117 @@ def getNextTID(self): :returns: The next unique transaction identifier ''' - tid = (ModbusTransactionManager.__tid + 1) & 0xffff - ModbusTransactionManager.__tid = tid - return tid + self.tid = (self.tid + 1) & 0xffff + return self.tid - def resetTID(self): + def reset(self): ''' Resets the transaction identifier ''' - ModbusTransactionManager.__tid = Defaults.TransactionId + self.tid = Defaults.TransactionId + self.transactions = type(self.transactions)() + + +class DictTransactionManager(ModbusTransactionManager): + ''' Impelements a transaction for a manager where the + results are keyed based on the supplied transaction id. + ''' + + def __init__(self, client): + ''' Initializes an instance of the ModbusTransactionManager + + :param client: The client socket wrapper + ''' + self.transactions = {} + super(DictTransactionManager, self).__init__(client) + + def __iter__(self): + ''' Iterater over the current managed transactions + + :returns: An iterator of the managed transactions + ''' + return iter(self.transactions.keys()) + + def addTransaction(self, request, tid=None): + ''' Adds a transaction to the handler + + This holds the requets in case it needs to be resent. + After being sent, the request is removed. + + :param request: The request to hold on to + :param tid: The overloaded transaction id to use + ''' + tid = tid if tid != None else request.transaction_id + _logger.debug("adding transaction %d" % tid) + self.transactions[tid] = request + + def getTransaction(self, tid): + ''' Returns a transaction matching the referenced tid + + If the transaction does not exist, None is returned + + :param tid: The transaction to retrieve + ''' + _logger.debug("getting transaction %d" % tid) + return self.transactions.pop(tid, None) + + def delTransaction(self, tid): + ''' Removes a transaction matching the referenced tid + + :param tid: The transaction to remove + ''' + _logger.debug("deleting transaction %d" % tid) + self.transactions.pop(tid, None) + + +class FifoTransactionManager(ModbusTransactionManager): + ''' Impelements a transaction for a manager where the + results are returned in a FIFO manner. + ''' + + def __init__(self, client): + ''' Initializes an instance of the ModbusTransactionManager + + :param client: The client socket wrapper + ''' + super(FifoTransactionManager, self).__init__(client) + self.transactions = [] + + def __iter__(self): + ''' Iterater over the current managed transactions + + :returns: An iterator of the managed transactions + ''' + return iter(self.transactions) + + def addTransaction(self, request, tid=None): + ''' Adds a transaction to the handler + + This holds the requets in case it needs to be resent. + After being sent, the request is removed. + + :param request: The request to hold on to + :param tid: The overloaded transaction id to use + ''' + tid = tid if tid != None else request.transaction_id + _logger.debug("adding transaction %d" % tid) + self.transactions.append(request) + + def getTransaction(self, tid): + ''' Returns a transaction matching the referenced tid + + If the transaction does not exist, None is returned + + :param tid: The transaction to retrieve + ''' + _logger.debug("getting transaction %s" % str(tid)) + return self.transactions.pop(0) if self.transactions else None + + def delTransaction(self, tid): + ''' Removes a transaction matching the referenced tid + + :param tid: The transaction to remove + ''' + _logger.debug("deleting transaction %d" % tid) + if self.transactions: self.transactions.pop(0) #---------------------------------------------------------------------------# @@ -782,7 +865,8 @@ def _filter(a): # Exported symbols #---------------------------------------------------------------------------# __all__ = [ - "ModbusTransactionManager", + "FifoTransactionManager", + "DictTransactionManager", "ModbusSocketFramer", "ModbusRtuFramer", "ModbusAsciiFramer", "ModbusBinaryFramer", ] diff --git a/test/test_transaction.py b/test/test_transaction.py index 867ce154c..f675b1fa9 100644 --- a/test/test_transaction.py +++ b/test/test_transaction.py @@ -15,12 +15,14 @@ class ModbusTransactionTest(unittest.TestCase): #---------------------------------------------------------------------------# def setUp(self): ''' Sets up the test environment ''' + self.client = None self.decoder = ServerDecoder() - self._manager = ModbusTransactionManager() self._tcp = ModbusSocketFramer(decoder=self.decoder) self._rtu = ModbusRtuFramer(decoder=self.decoder) self._ascii = ModbusAsciiFramer(decoder=self.decoder) self._binary = ModbusBinaryFramer(decoder=self.decoder) + self._manager = DictTransactionManager(self.client) + self._queue_manager = FifoTransactionManager(self.client) def tearDown(self): ''' Cleans up the test environment ''' @@ -29,20 +31,20 @@ def tearDown(self): del self._rtu del self._ascii - #---------------------------------------------------------------------------# - # Other Class tests - #---------------------------------------------------------------------------# - def testModbusTransactionManagerTID(self): - ''' Test the tcp transaction manager TID ''' + #---------------------------------------------------------------------------# + # Dictionary based transaction manager + #---------------------------------------------------------------------------# + def testDictTransactionManagerTID(self): + ''' Test the dict transaction manager TID ''' for tid in range(1, self._manager.getNextTID() + 10): - self.assertEqual(tid+2, self._manager.getNextTID()) - self._manager.resetTID() + self.assertEqual(tid+1, self._manager.getNextTID()) + self._manager.reset() self.assertEqual(1, self._manager.getNextTID()) - def testGetTransactionManagerTransaction(self): - ''' Test the tcp transaction manager ''' + def testGetDictTransactionManagerTransaction(self): + ''' Test the dict transaction manager ''' class Request: pass - self._manager.resetTID() + self._manager.reset() handle = Request() handle.transaction_id = self._manager.getNextTID() handle.message = "testing" @@ -50,10 +52,10 @@ class Request: pass result = self._manager.getTransaction(handle.transaction_id) self.assertEqual(handle.message, result.message) - def testDeleteTransactionManagerTransaction(self): - ''' Test the tcp transaction manager ''' + def testDeleteDictTransactionManagerTransaction(self): + ''' Test the dict transaction manager ''' class Request: pass - self._manager.resetTID() + self._manager.reset() handle = Request() handle.transaction_id = self._manager.getNextTID() handle.message = "testing" @@ -62,7 +64,40 @@ class Request: pass self._manager.delTransaction(handle.transaction_id) self.assertEqual(None, self._manager.getTransaction(handle.transaction_id)) - #---------------------------------------------------------------------------# + #---------------------------------------------------------------------------# + # Queue based transaction manager + #---------------------------------------------------------------------------# + def testFifoTransactionManagerTID(self): + ''' Test the fifo transaction manager TID ''' + for tid in range(1, self._queue_manager.getNextTID() + 10): + self.assertEqual(tid+1, self._queue_manager.getNextTID()) + self._queue_manager.reset() + self.assertEqual(1, self._queue_manager.getNextTID()) + + def testGetFifoTransactionManagerTransaction(self): + ''' Test the fifo transaction manager ''' + class Request: pass + self._queue_manager.reset() + handle = Request() + handle.transaction_id = self._queue_manager.getNextTID() + handle.message = "testing" + self._queue_manager.addTransaction(handle) + result = self._queue_manager.getTransaction(handle.transaction_id) + self.assertEqual(handle.message, result.message) + + def testDeleteFifoTransactionManagerTransaction(self): + ''' Test the fifo transaction manager ''' + class Request: pass + self._queue_manager.reset() + handle = Request() + handle.transaction_id = self._queue_manager.getNextTID() + handle.message = "testing" + + self._queue_manager.addTransaction(handle) + self._queue_manager.delTransaction(handle.transaction_id) + self.assertEqual(None, self._queue_manager.getTransaction(handle.transaction_id)) + + #---------------------------------------------------------------------------# # TCP tests #---------------------------------------------------------------------------# def testTCPFramerTransactionReady(self):