Refactoring, type annotations and fixes

[ngjunsiang]

This pull request covers a number of changes:

Use DataQueryDatumSpecification and DatumSpecification in composition

This implements the change proposed in #49 (comment)

Before:

open-dis-python/opendis/dis7.py

Lines 6 to 22 in 3a52c4f

	class DataQueryDatumSpecification( object ):
	"""List of fixed and variable datum records. Section 6.2.17 """
	def __init__(self,
	numberOfFixedDatums=0,
	numberOfVariableDatums=0,
	fixedDatumIDs=None,
	variableDatumIDs=None):
	""" Initializer for DataQueryDatumSpecification"""
	self.numberOfFixedDatums = numberOfFixedDatums
	""" Number of fixed datums"""
	self.numberOfVariableDatums = numberOfVariableDatums
	""" Number of variable datums"""
	self.fixedDatumIDs = fixedDatumIDs or []
	""" variable length list fixed datum IDs"""
	self.variableDatumIDs = variableDatumIDs or []
	""" variable length list variable datum IDs"""

After:

open-dis-python/opendis/dis7.py

Lines 6 to 24 in b4ccb3d

	class DataQueryDatumSpecification(object):
	"""Number and identification of fixed and variable datum records. Section 6.2.17"""
	def __init__(self,
	fixedDatumIDs=None,
	variableDatumIDs=None):
	"""Initializer for DataQueryDatumSpecification"""
	self.fixedDatumIDs = fixedDatumIDs or []
	"""variable length list fixed datum IDs"""
	self.variableDatumIDs = variableDatumIDs or []
	"""variable length list variable datum IDs"""
	@property
	def numberOfFixedDatumIDs(self) -> int:
	return len(self.fixedDatumIDs)
	@property
	def numberOfVariableDatumIDs(self) -> int:
	return len(self.variableDatumIDs)

Before:

open-dis-python/opendis/dis7.py

Lines 5042 to 5067 in 3a52c4f

	class ActionRequestPdu( SimulationManagementFamilyPdu ):
	"""Section 7.5.7. Request from simulation manager to a managed entity to perform a specified action. COMPLETE"""
	def __init__(self,
	requestID=0,
	actionID=0,
	numberOfFixedDatumRecords=0,
	numberOfVariableDatumRecords=0,
	fixedDatumRecords=None,
	variableDatumRecordList=None):
	""" Initializer for ActionRequestPdu"""
	super(ActionRequestPdu, self).__init__()
	self.requestID = requestID
	""" identifies the request being made by the simulaton manager"""
	self.actionID = actionID
	""" identifies the particular action being requested(see Section 7 of SISO-REF-010)."""
	self.numberOfFixedDatumRecords = numberOfFixedDatumRecords
	""" Number of fixed datum records"""
	self.numberOfVariableDatumRecords = numberOfVariableDatumRecords
	""" Number of variable datum records"""
	self.fixedDatumRecords = fixedDatumRecords or []
	""" variable length list of fixed datums"""
	self.variableDatumRecords = variableDatumRecords or []
	""" variable length list of variable length datums"""
	self.pduType = 16
	""" initialize value """

After:

open-dis-python/opendis/dis7.py

Lines 4781 to 4815 in f62b7e3

	class ActionRequestPdu(SimulationManagementFamilyPdu):
	"""Section 7.5.7. Request from simulation manager to a managed entity to perform a specified action. COMPLETE"""
	def __init__(self,
	requestID=0,
	actionID=0,
	fixedDatumRecords=None,
	variableDatumRecords=None):
	"""Initializer for ActionRequestPdu"""
	super(ActionRequestPdu, self).__init__()
	self.requestID = requestID
	"""identifies the request being made by the simulaton manager"""
	self.actionID = actionID
	"""identifies the particular action being requested(see Section 7 of SISO-REF-010)."""
	# Use DatumSpecification
	self._datums = DatumSpecification(fixedDatumRecords or [],
	variableDatumRecords or [])
	self.pduType = 16
	"""initialize value"""
	@property
	def numberOfFixedDatumRecords(self) -> int:
	return self._datums.numberOfFixedDatumRecords
	@property
	def numberOfVariableDatumRecords(self) -> int:
	return self._datums.numberOfVariableDatumRecords
	@property
	def fixedDatumRecords(self) -> list["FixedDatum"]:
	return self._datums.fixedDatumRecords
	@property
	def variableDatumRecords(self) -> list["VariableDatum"]:
	return self._datums.variableDatumRecords

Any fixes to DataQueryDatumSpecification and DatumSpecification will not need to be copied to the classes that use it.

Style fixes

• removed inheritance from object which is a Python2 requirement; Python 3 no longer requires it
• edited most class docstrings to follow PEP257 conventions
• removed redundant comments and whitespace
• moved shared attributes (pduType, protocolFamily) from init to class attribute.

Bugfixes

Some parsing code I found to be non-compliant with IEEE 1278.1-2012 has been fixed. Also replaced a few null() calls with calls to the appropriate classes (addressing #14, but not yet completely fixed)

Add type annotations

This implements the change proposed in #49 (comment)

Type aliases

open-dis-python/opendis/dis7.py

Lines 5 to 19 in 76dedb4

	# Type aliases (for readability)
	enum8 = int
	enum16 = int
	enum32 = int
	int16 = int
	int32 = int
	uint8 = int
	uint16 = int
	uint32 = int
	uint64 = int
	float32 = float
	float64 = float
	struct8 = bytes
	struct16 = bytes
	struct32 = bytes

The above type aliases are equivalent to the Python classes (int, float, bytes) assigned to them, and are used in type annotations to add more information about the required parameter type (enum, record (struct), signed or unsigned int, float) and size.

Type annotations have been added to most init parameters (some tricky ones are still unannotated), and it should be pretty clear from the annotations what is required now. In supported IDEs, these annotations can also be read and displayed in help text:

Future work

This is groundwork for future pull requests aimed at organising and structuring the codebase, so as to make it easier to address the open issues.

In particular, the current code structure and flow (instantiating a single PDU class to wrap an input/output stream for serialization/parsing) makes it tricky to handle bitfields and sub-enumerations. I'm working on a parallel effort, a code generator for enumerations from SISO-REF-010 which can be used with this package to provide introspection (auto-lookup of enum values) and unpack bitfields.

[ngjunsiang]

While this pull request is open, another quick question. I found this in the codebase:

open-dis-python/opendis/dis7.py

Lines 273 to 286 in 76dedb4

	class UnsignedDISInteger:
	"""container class not in specification"""
	def __init__(self, val=0):
	self.val = val
	"""unsigned integer"""
	def serialize(self, outputStream):
	"""serialize the class"""
	outputStream.write_unsigned_int(self.val)
	def parse(self, inputStream):
	"""Parse a message. This may recursively call embedded objects."""
	self.val = inputStream.read_unsigned_int()

This class appears to be unused. I have left it in the codebase for now. If it is also legacy code (in the same category as the *Chunk classes), let me know and I'll push another commit to remove it.

[leif81]

Thank-you @ngjunsiang
I'm going to be unavailable for the next week but I'll provide some feedback when I'm back

[ngjunsiang]

quick ping @leif81 to see if there's anything else you need to see for this PR?

[leif81]

Have started to review, I should be able to finish up in next couple days.

[leif81]

@ngjunsiang the change is very large (>7000 lines changed), which makes it very difficult and time consuming to review. But I feel good about what I'm seeing. I will proceed with merging it. Thank-you for the improvement