Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

UBXMessage.config_set() throws TypeError for config item of X1 type #172

Open
fronders opened this issue Dec 27, 2024 · 2 comments
Open
Assignees
Labels
expected behaviour Working as expected

Comments

@fronders
Copy link

fronders commented Dec 27, 2024

Describe the bug

UBXMessage.config_set() tries to lookup attribute types and fails because of val2bytes() function for att to valb conversion

Issue is caused by code on line 281:

def val2bytes(val, att: str) -> bytes:
"""
Convert value to bytes for given UBX attribute type.
:param object val: attribute value e.g. 25
:param str att: attribute type e.g. 'U004'
:return: attribute value as bytes
:rtype: bytes
:raises: UBXTypeError
"""
if att == ubt.CH: # single variable-length string (e.g. INF-NOTICE)
return val.encode("utf-8", "backslashreplace")
atts = attsiz(att)
if atttyp(att) in ("C", "X"): # byte or char
valb = val
elif atttyp(att) in ("E", "L", "U"): # unsigned integer
valb = val.to_bytes(atts, byteorder="little", signed=False)
elif atttyp(att) == "A": # array of unsigned integers
atts = attsiz(att)
valb = b""
for i in range(atts):
valb += val[i].to_bytes(1, byteorder="little", signed=False)
elif atttyp(att) == "I": # signed integer
valb = val.to_bytes(atts, byteorder="little", signed=True)
elif att == ubt.R4: # single precision floating point
valb = struct.pack("<f", val)
elif att == ubt.R8: # double precision floating point
valb = struct.pack("<d", val)
else:
raise ube.UBXTypeError(f"Unknown attribute type {att}")
return valb

which returns integer instead of bytes in case of bitmask attribute.

Adding to_bytes() conversion fixes the issue.
This is especially critical for X8 type, where little-endian conversion is a must.
I don't see why X1/2/4/8 attributes need to be treated differently from U1/2/4/8 ones

Info

  1. The pyubx2 version 1.2.48
  2. The complete Python script.
import pyubx2

command = pyubx2.UBXMessage.config_set(
    layers=pyubx2.SET_LAYER_FLASH,
    transaction=pyubx2.TXN_NONE,
    cfgData=[(0x20920006,0)]
)
print(command)
  1. The error message and full traceback.
Traceback (most recent call last):
  File "D:\test\ublox\ublox_test.py", line 3, in <module>
    command = pyubx2.UBXMessage.config_set(
  File "D:\test\.venv\lib\site-packages\pyubx2\ubxmessage.py", line 742, in config_set
    lis = lis + keyb + valb
TypeError: can't concat int to bytes
  1. Expected Behaviour: code prints
<UBX(CFG-VALSET, version=0, ram=0, bbr=0, flash=1, action=0, reserved0=0, CFG_INFMSG_NMEA_I2C=b'\x00')>

To Reproduce

Steps to reproduce the behaviour:

  1. Try to compose any CFG-VALSET item with "X1" or "X8" type

Desktop:

  • Windows 11 x64.
  • UART1 connection via USB-UART dongle.

GNSS/GPS Device:

  • Device Model/Generation: ublox NEO-F10N
  • Firmware Version: SPG 6.00
  • Protocol: 40.00
@fronders fronders changed the title UBXMessage.config_set() throws ValueType exception for config item of X1 type UBXMessage.config_set() throws TypeError for config item of X1 type Dec 27, 2024
@semuadmin semuadmin added the expected behaviour Working as expected label Dec 28, 2024
@semuadmin
Copy link
Contributor

semuadmin commented Dec 28, 2024

Hi @fronders,

This is expected behaviour for X type attributes. The UBX protocol makes an intentional distinction between U (unsigned integer) and X (byte) types, and pyubx2 has to observe this distinction. Not all X type attributes can be interpreted as unsigned integers (the majority are actually character strings), so a .to_bytes() conversion would not be appropriate in all cases.

In this case the issue can be simply resolved by supplying the value as bytes, i.e.:

import pyubx2

command = pyubx2.UBXMessage.config_set(
    layers=pyubx2.SET_LAYER_FLASH,
    transaction=pyubx2.TXN_NONE,
    cfgData=[(0x20920006, b"\x00")],
)
print(command)
<UBX(CFG-VALSET, version=0, ram=0, bbr=0, flash=1, action=0, reserved0=0, CFG_INFMSG_NMEA_I2C=b'\x00')>

As to why some config database values are defined as X when a U (or C) type would seem more intuitive - I'm afraid you'll have to ask u-blox that question!

@fronders
Copy link
Author

fronders commented Dec 28, 2024

OK, fair enough, I see many X8 ones are char sequences, while X1, X2 and X4 mostly are bitmasks (which should be treated as unsigned imo). But at least the fact that you need to pass little-endian bytes to cfgData needs to be documented somewhere :) The val2bytes(val, att) behavior is a bit inconsistent then, as one would expect it to return bytes. Maybe check for incoming val type and raise an exception telling user to pass bytes instead of dying silently?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
expected behaviour Working as expected
Projects
None yet
Development

No branches or pull requests

2 participants