ethz-asl / ethzasl_xsens_driver

Driver for xsens IMUs
BSD 2-Clause "Simplified" License
102 stars 112 forks source link

Synchronization setting allows for negative numbers but driver code does not? #81

Closed chutsu closed 3 years ago

chutsu commented 6 years ago

The Xsens IMU allows one to offset the synchronization setting with a negative offset (doc here):

Synchronization settings:
    The format follows the xsens protocol documentation. All fields are required
    and separated by commas.
    Note: The entire synchronization buffer is wiped every time a new one
          is set, so it is necessary to specify the settings of multiple
          lines at once.
...

        Offset:
            Offset from event to pulse generation.
            100 microseconds unit, range [-30000...+30000]

The struct.pack() format code in data = b''.join(struct.pack('!BBBBHHHH', *sync_setting) of:

def SetSyncSettings(self, sync_settings):
        """Set the synchronisation settings (mark IV)"""
        self._ensure_config_state()
        data = b''.join(struct.pack('!BBBBHHHH', *sync_setting)
                        for sync_setting in sync_settings)
        self.write_ack(MID.SetSyncSettings, data)

in Line 342 of mtdevice.py. Should the last element not be h and not H to allow for a negative number? (format code doc here)

fcolas commented 6 years ago

Ah, good catch. Actually it's more subtle than that as it depends whether it's a Delay, Clock period, or Offset value. All three can go at that eighth spot (and I realize that the command line documentation is not clear on that) but they behave differently:

I'll have to add a bit of logic here. Thanks for spotting this.