Maksim Salau <maksim.sa...@gmail.com> wrote: > Modbus registers may be referenced by address or by a number. > Number defines both register type (coil, discrete input, input > register and holding register) and its address. > > Two conversions are used simultaneously: > 1. For registers with address less than 9999 offset of x0001 is > used, where x defines the register type; 2. Offset x00001 is > used. This convention is applicable for any address. > > The change adds a single point of conversion of addresses into > register numbers and fixes conversion done in different > handlers which were not in sync with each other.
While unifying the code that handles this is good, you're keeping the term "address" but switching it's implementation to "register number". I would suggest either a) leaving the address terms, and therefore leaving them as zero based, or b) changing the terminology to register number. Personally, I would prefer option (a) but as long as it's consistent... Further, using the "ranged" notation 3xxxx and 4xxxxx it's expected to see them in decimal, not hex, but that was an existing quirk, really, you've just kept it here. Sincerely, Karl Palsson > --- > decoders/modbus/pd.py | 67 ++++++++++++++++++++++++------------------- > 1 file changed, 38 insertions(+), 29 deletions(-) > > diff --git a/decoders/modbus/pd.py b/decoders/modbus/pd.py > index 81aaf2a..76891bb 100644 > --- a/decoders/modbus/pd.py > +++ b/decoders/modbus/pd.py > @@ -180,6 +180,33 @@ def calc_crc(self, last_byte): > byte2 = (result & 0xFF00) >> 8 > return (byte1, byte2) > > + def get_register_number(self, function, address): > + # Register number defines both type and address of a register > + # > https://en.wikipedia.org/wiki/Modbus#Coil,_discrete_input,_input_register,_holding_register_numbers_and_addresses > + ENTITY_TYPES = {1: 0, # Read coils > + 5: 0, # Write Single Coil > + 15: 0, # Write Multiple Coils > + 2: 1, # Read Discrete Inputs > + 4: 3, # Read Input Registers > + 3: 4, # Read Multiple Holding Registers > + 6: 4, # Write Single Holding Register > + 16: 4, # Write Multiple Holding Registers > + 22: 4, # Mask Write Register > + 23: 4, # Read/Write Multiple Registers > + 24: 4, # Read FIFO Queue > + } > + > + if function not in ENTITY_TYPES: > + return '-' > + > + entity_type = ENTITY_TYPES[function] > + if address <= 9998: > + register_number = '{:d}{:04d}'.format(entity_type, address + 1) > + else: > + register_number = '{:d}{:05d}'.format(entity_type, address + 1) > + > + return register_number > + > def parse_write_single_coil(self): > '''Parse function 5, write single coil.''' > self.minimum_length = 8 > @@ -188,7 +215,7 @@ def parse_write_single_coil(self): > > address = self.half_word(2) > self.puti(3, 'address', > - 'Address 0x{:X} / {:d}'.format(address, address + 10000)) > + 'Address 0x{:X} / {}'.format(address, > self.get_register_number(5, address))) > > raw_value = self.half_word(4) > value = 'Invalid Coil Value' > @@ -208,7 +235,7 @@ def parse_write_single_register(self): > > address = self.half_word(2) > self.puti(3, 'address', > - 'Address 0x{:X} / {:d}'.format(address, address + 30000)) > + 'Address 0x{:X} / {}'.format(address, > self.get_register_number(6, address))) > > value = self.half_word(4) > value_formatted = 'Register Value 0x{0:X} / {0:d}'.format(value) > @@ -261,7 +288,7 @@ def parse_mask_write_register(self): > > address = self.half_word(2) > self.puti(3, 'address', > - 'Address 0x{:X} / {:d}'.format(address, address + 30001)) > + 'Address 0x{:X} / {}'.format(address, > self.get_register_number(22, address))) > > self.half_word(4) # To make sure we don't oveflow data. > and_mask_1 = data[4].data > @@ -470,22 +497,17 @@ def parse_write_multiple(self): > if function == 15: > data_unit = 'Coils' > max_outputs = 0x07B0 > - long_address_offset = 10001 > elif function == 16: > data_unit = 'Registers' > max_outputs = 0x007B > - long_address_offset = 30001 > > self.puti(1, 'function', > 'Function {}: Write Multiple {}'.format(function, data_unit)) > > starting_address = self.half_word(2) > - # Some instruction manuals use a long form name for addresses, this > is > - # listed here for convienience. > - address_name = long_address_offset + starting_address > self.puti(3, 'address', > - 'Start at address 0x{:X} / {:d}'.format(starting_address, > - address_name)) > + 'Start at address 0x{:X} / {}'.format(starting_address, > + > self.get_register_number(function, starting_address))) > > quantity_of_outputs = self.half_word(4) > if quantity_of_outputs <= max_outputs: > @@ -646,13 +668,9 @@ def parse_read_data_command(self): > 'Function {}: {}'.format(function, functionname)) > > starting_address = self.half_word(2) > - # Some instruction manuals use a long form name for addresses, this > is > - # listed here for convienience. > - # Example: holding register 60 becomes 30061. > - address_name = 10000 * function + 1 + starting_address > self.puti(3, 'address', > - 'Start at address 0x{:X} / {:d}'.format(starting_address, > - address_name)) > + 'Start at address 0x{:X} / {}'.format(starting_address, > + > self.get_register_number(function, starting_address))) > > self.puti(5, 'length', > 'Read {:d} units of data'.format(self.half_word(4))) > @@ -681,23 +699,18 @@ def parse_write_multiple(self): > data_unit = 'Coils' > max_outputs = 0x07B0 > ratio_bytes_data = 1/8 > - long_address_offset = 10001 > elif function == 16: > data_unit = 'Registers' > max_outputs = 0x007B > ratio_bytes_data = 2 > - long_address_offset = 30001 > > self.puti(1, 'function', > 'Function {}: Write Multiple {}'.format(function, data_unit)) > > starting_address = self.half_word(2) > - # Some instruction manuals use a long form name for addresses, this > is > - # listed here for convienience. > - address_name = long_address_offset + starting_address > self.puti(3, 'address', > - 'Start at address 0x{:X} / {:d}'.format(starting_address, > - address_name)) > + 'Start at address 0x{:X} / {}'.format(starting_address, > + > self.get_register_number(function, starting_address))) > > quantity_of_outputs = self.half_word(4) > if quantity_of_outputs <= max_outputs: > @@ -778,13 +791,9 @@ def parse_read_write_registers(self): > self.puti(1, 'function', 'Function 23: Read/Write Multiple > Registers') > > starting_address = self.half_word(2) > - # Some instruction manuals use a long form name for addresses, this > is > - # listed here for convienience. > - # Example: holding register 60 becomes 30061. > - address_name = 30001 + starting_address > self.puti(3, 'address', > - 'Read starting at address 0x{:X} / {:d}'.format(starting_address, > - address_name)) > + 'Read starting at address 0x{:X} / {}'.format(starting_address, > + > self.get_register_number(function, starting_address))) > > self.puti(5, 'length', 'Read {:d} units of > data'.format(self.half_word(4))) > > -- > 2.25.1 > > > > _______________________________________________ > sigrok-devel mailing list > sigrok-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/sigrok-devel
OpenPGP-digital-signature.html
Description: OpenPGP Digital Signature
_______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
