CCSDSPy / ccsdspy

I/O interface and utilities for CCSDS binary spacecraft data in Python. Library used in flight missions at NASA, NOAA, and SWRI
https://ccsdspy.org
BSD 3-Clause "New" or "Revised" License
78 stars 19 forks source link

Signed field not on byte boundaries uses too many bits to calculate sign #80

Closed scandey closed 1 year ago

scandey commented 1 year ago

For example: the 12 bit signed integer 0x800 returns -30720, or 0x800 - 0x8000. Looks like all integer fields are fit into the nearest numpy size that fits the required bits and sign extension is not working properly?

Example code:

# %%
import ccsdspy
from ccsdspy import PacketField, PacketArray
import io

# %%
pkt = ccsdspy.FixedLength([
    PacketField(name='uintfive', data_type='uint', bit_length=3),
    PacketField(name='negfive', data_type='int', bit_length=5),
    PacketField(name='postwelve', data_type='int', bit_length=12),
    PacketField(name='negtwelve', data_type='int', bit_length=12),
])
# %%
# 0b101, 0b11011, 0b000000001100, 0b111111110100
fakepkt = io.BytesIO(b"\x00\x01\xC0\x00\x00\x03\x5B\x00\xCF\xFA")
print(f'Fake twelve single file has {fakepkt.getbuffer().nbytes} bytes')
with open('fake_off_byte_fields.ccsds', 'wb') as file:
    file.write(fakepkt.getbuffer())

# %%
print(f'Attempting to load just one packet from file')
pkt.load('fake_off_byte_fields.ccsds')
# returns {'uintfive': array([2], dtype=uint8), 'negfive': array([-101], dtype=int8), 
#         'postwelve': array([12], dtype='>i2'), 'negtwelve': array([-28678], dtype='>i2')}
ddasilva commented 1 year ago

Thanks for reporting! I'll take a look at this tomorrow. There was a patch last week made to odd length negative int handling, which is probably related.

ddasilva commented 1 year ago

I think in your test data, your uintfive is actually 2 and negtwelve is actually -6. Can you double check this?

In [1]: from bitstring import BitArray

In [2]: arr = BitArray(bytes=b"\x00\x01\xC0\x00\x00\x03\x5B\x00\xCF\xFA")

In [3]: arr[48:51].uint
Out[3]: 2

In [4]: arr[-12:].int
Out[4]: -6

If that's is indeed the case, then the patch I pushed to gh-issue-78-fix branch should fix the issue.

Fake twelve single file has 10 bytes
Attempting to load just one packet from file
{'negfive': array([-5], dtype=int8),
 'negtwelve': array([-6], dtype=int16),
 'postwelve': array([12], dtype=int16),
 'uintfive': array([2], dtype=uint8)}

Best, Daniel

scandey commented 1 year ago

Totally right on both counts, that'll teach me to do binary conversions by hand late at night! Confirmed that it works on my larger files too. Thank you!

ddasilva commented 1 year ago

Fixed in #81