rscada / python-mbus

Python wrapper for libmbus
BSD 3-Clause "New" or "Revised" License
25 stars 28 forks source link

MBusFrame._fields_.start1 wrong size #25

Open Cougar opened 5 years ago

Cougar commented 5 years ago

Hi,

Is this a typo or why is start1 type c_uint8 * 16 in MBusFrame._fields_? According to https://github.com/rscada/libmbus/blob/master/mbus/mbus-protocol.h#L77 it shoul also be just c_uint8.

typedef struct _mbus_frame {
    unsigned char start1;
    unsigned char length1;
    unsigned char length2;
    unsigned char start2;
...

How do you fill and use the frame structure with such array definition?

I locally changed it to c_uint8

diff --git a/mbus/MBusFrame.py b/mbus/MBusFrame.py
index 9d1b2b5..8f511b6 100644
--- a/mbus/MBusFrame.py
+++ b/mbus/MBusFrame.py
@@ -7,7 +7,7 @@ class MBusFrame(Structure):
         return "MBusFrame: XXX"

 MBusFrame._fields_ = [
-        ("start1",   c_uint8 * 16),  # MBusFrameFixed
+        ("start1",   c_uint8),
         ("length1",  c_uint8),
         ("length2",  c_uint8),
         ("start2",   c_uint8),

Now I can easily create MBus frames with code like this:

        mbus_frame = MBusFrame()
        mbus_frame.start1 = MBUS_FRAME_LONG_START
        mbus_frame.length1 = data_size + 3 
        mbus_frame.length2 = mbus_frame.length1
        mbus_frame.start2 = mbus_frame.start1
...

Is this a mistake and should I send a pull request for that or is it possible to use this structure in any other way?

qris commented 5 years ago

Agreed, and fixed in https://github.com/qris/python-mbus/commit/236b82f9c544409eeeb9701af58170fa65394809 along with another field that's wrong, making next invalid and multi-frame replies unreadable.