riverlane/QHAL

command creator returns wrong results when angle is negative

Closed this issue · 3 comments

UQ HAL has been complaining when command packer is used with negative angles. There might be an issue with the below snippet form _commands.py

cmd = (
(opcode.code << Shifts.OPCODE.value)
| (arg0 << Shifts.ARG0.value)
| qidx0
)

when arg0 is negative and bitwise or is performed, all the prepended 1's to represent the negative number ( 2's complement ) overrides fields before it ( bitwise or with 1s ). This results in a wrong packed result being returned from command creator.
There are other few places where similar bitwise operations are performed in the codebase which would need changing.

One way to fix this would be to bitwise mask each field before the bitwise or.

In case the plan is not to never allow negative angles, I wonder if we could somehow detect negative angles and return an error message back to the user...

If angle_binary_representation is used to convert the angle to the appropriate integer, this issue shouldn't arise (all angles will be converted to an angle between 0 and 2pi). Currently, 16 bits are used for the angle representation by dividing the 0-2pi range into 2**16 discrete values. Without changing this, it won't be possible to detect a negative angle.
If we wanted to allow negative angles, we could use 1 bit for the sign and 15 for the value.

def angle_binary_representation(angle: float) -> int:
    """Converts an angle in radians to a 16-bit representation.
    Parameters
    ----------
    angle : float
        The angle (in radians) to be converted.
    Returns
    -------
    int
        16-bit representation of angle.
    """
    return int(np.rint((angle % (2*np.pi)) / (2 * np.pi / 2**16)))

Hi @can-uq thanks for raising this issue. If it still needs addressing, can you please create a new issue in the new repo? Thanks!