DeviceHandlerBase: All Refactoring #44
However, the changes discussed on 01.04 includes more than just DeviceCommunicationIF, so maybe a new issue would be good to discuss all the changes I coded.
Repitition of points from #19:
- Replace the open call and allocate the cookies in the factory and hand the pointer to the DeviceCommunicationIF
- Add a request length for requestReceiveMessage
- Find better names for all functions
- Document the interaction patterns
Unfortunately, there are some several API changes after looking at this topic and implementing the SPI and I2C communication interfaces for SOURCE.
I also found out that an separate initialization function called in the DeviceHandlerBase (that was
open(), I renamed it to
initializeInterface()) is still needed because I need to initialize data structures in the communication interface implementation with information stored inside the cookie (e.g. for a map of addresses to Com Status information).
Here is a list of the various changes. I have several branches which implemented subsets of these changes, but I think it would be best to gather them into one pull request as they are connected to each other.
Some clarification required:
What is the meaning of the returnvalues located in DHB, and the ones in DeviceHandlerIF? Did I understand correctly that the returnvalues in DHB alter the behaviour of DHB while the returnvalues in DeviceHandlerIF are just error codes? If it is supposed to be like that, I suggest moving the rest of the error codes (2-3 left I think) to DeviceHandlerIF too.
UPDATE: Actually, the returnvalues of DHB are "private" to DHB, so its okay like that.
How to integrate change: Rename Cookie to CookieIF
No diff for header file , needs to be diffed from command line.
- Does not take and cache ioBoardAddress and maxReplyLength anymore. It is assumed that these variables are passed to the Cookie in the factory construction
- Takes a CookieIF pointer, which is cached.
switchCookieChannelfunction commented out for now
- DeviceReplyMap has a new entry called replyLen.
Its value can be set in insertInCommandReplyMap on device handler initialization (or updateCommandReplyMap). It is used to pass the correct replyLength in the
requestReceiveMessage()call to the communication interface.
Documentation / Formatting / Code Style etc.
- Functions in header resorted in order of importance for someone implementing child handlers. The abstract functions are closer at the top now as these must be implemented. I grouped together insertInCommandAndReplyMap and its helper functions.
- getSwitches and getTransitionDelay closer to the top too.
- additional documentation added or corrected for some functions. There was an explanation in
DeviceHandlerBase.hI did not understand (line 350).
- reduced size of constructor by moving 0 or nullptr initialization (NULL replaced by nullptr) into the header.
- I reduced the number of includes and sorted them a bit.
- new returnvalue IGNORE_FULL_PACKET for scanForReply() function. If this is returned, DHB will stop parsing and ignore the reply
- performOperationHook() : A virtual hook function which will be called once per performOperation. Default implementation empty.
- debugInterface() a debug interface. If I put a printout into the DHB, it will be printed for each child handler. If I implement the debugInterface() in the respective child handler (default implementation is empty), I can limit printouts to the target child handler to debug. This can be also useful for breakpoints. I can just set the breakpoint in the child handler debugInterface() to only get stepping for a target child handler.
How to integrate change
- Cookies are now instantiated in the factory instead of the communication interface
- CommunicationIF specific configuration can be performed in the
- The DHB constructor arguments maxReplyLen and ioBoardAddress were replaced by CookieIF, which can also contain these values, depending on cookie configuration, so all calls to DHB have to be adapted.
getAddress()removed for now
sendMessage()data pointer is constant
requestReceiveMessage()also supplies a replyLength in addition to the cookie. If a sequential communication system is used, the correct replyLength should be taken from the reply map and will be passed to the communication interface. Otherwise, 0 is passed and the communication interface can device to read everything or nothing.
- I added a new returnvalue
NO_REPLY_RECEIVED, which can be used in
receiveRequestedReply()to explicitely state that no reply was received.
How to integrate change
- The removed functions don't have to be implemented anymore.
- open() calls need to be renamed to initializeInterface(). A constructed cookie is passedf to the ComIF, which can then be modified or used to initialize comIF specific parameters or data structures.
- Input arguments of some other abstract functions have changed.
- Cookie initialization moved from Child com interface to factory.
I think there was some issue with the indentation.
I improved the readability of this file by writing the explanation
of the modes above the mode definitions and keeping to the 80 column widht limit
(hope that did not break code parsers).
I also renamed the RmapAction to CommunicationAction (more generic name).
Label: API Change, Documentation, Enhancement
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?