support for dlt-daemon 2.18.6?

[LocutusOfBorg]

Hello, looks like the testsuite is now broken with dlt-daemon 2.18.6.

Can you please have a look?

thanks

[aigarius]

We will take a look. They have a habit of changing a lot of internal interfaces in minor versions :(

[LocutusOfBorg]

unfortunately this is a big issue for Debian stable, we would really like to have a binding that works across different versions of dlt-daemon...

[aigarius]

There is a mechanism in the binding to detect libdlt version and present different binary interfaces based on that. So it can be backward compatible with multiple versions of libdlt, but when dlt-daemon does changes to their data structures in minor versions one can not write a binding that will anticipate that. I think we will have to lock down the dependencies and conflicts in Debian tighter to at least prevent users breaking the setups without knowing it.

It is not an issue for Debian stable as such, because we can select which two versions will be in stable release and make sure they work together. Security fixes should not cause such issues.

[blublup]

I just copy the core_2185.py to core_2186.py and aligned with 2.18.6 definition (DltReceiver):

~# diff core_2185.py core_2186.py
2c2
< """v2.18.5 specific class definitions"""
---
> """v2.18.6 specific class definitions"""
41a42
>         DltReceiverType  type;    /**< type of connection handle */
51a53
>                 ("type", ctypes.c_int),

[LocutusOfBorg]

@blublup I don't know your name/email, I tried to give credits for your patch https://github.com/bmwcarit/python-dlt/pull/27 Let me know if I have to update the authorship

[LocutusOfBorg]

--- a/tests/dlt_core_unit_tests.py
+++ b/tests/dlt_core_unit_tests.py
@@ -21,8 +21,8 @@ def setUp(self):
                          'cDltStandardHeaderExtra': 12,
                          'cDltExtendedHeader': 10,
                          'cDLTMessage': 120,
-                         'cDltReceiver': 64,
-                         'cDltClient': 128}
+                         'cDltReceiver': 72,
+                         'cDltClient': 136}

     def test_sizeof(self):
         for clsname, expected in self.size_map.items():
@@ -36,11 +36,11 @@ class TestImportSpecificVersion(unittest.TestCase):

     def setUp(self):
         self.original_api_version = dlt.core.API_VER
-        self.version_answer = b"2.18.5"
-        self.version_str = (b"DLT Package Version: 2.18.5 STABLE, Package Revision: v2.18.5_5_g33fbad1, "
-                            b"build on Sep  2 2020 11:55:50\n-SYSTEMD -SYSTEMD_WATCHDOG -TEST -SHM\n")
-        self.version_filename = "core_2185.py"
-        self.version_truncate_str = "2.18.5"
+        self.version_answer = b"2.18.6"
+        self.version_str = (b"DLT Package Version: 2.18.6 STABLE, Package Revision: v2.18.6_5_22715aec, "
+                            b"build on Jan  6 2021 11:55:50\n-SYSTEMD -SYSTEMD_WATCHDOG -TEST -SHM\n")
+        self.version_filename = "core_2186.py"
+        self.version_truncate_str = "2.18.6"
         self.version_truncate_filename = "core_2180.py"

         dlt.core.API_VER = None

This is what I added on top of your change

[blublup]

I identified another impacting update in dltlib: dlt_receiver_init takes an additionnal argument in the third place: DltReceiverType

I found a fork would be more simple to handle changes: https://github.com/bmwcarit/python-dlt/compare/master...blublup:2.18.6

I don't know the process for review/merge in bmwcarit/master.

By the way, I'm not sure I identified all the issue for 2.18.6

[blublup]

@blublup I don't know your name/email, I tried to give credits for your patch

27

Let me know if I have to update the authorship

I'm sorry, I was off last days, maybe my fork would help.

[LocutusOfBorg]

I think your patch is probably wrong... the C structure is:

typedef struct
{
    int32_t lastBytesRcvd;    /**< bytes received in last receive call */
    int32_t bytesRcvd;        /**< received bytes */
    int32_t totalBytesRcvd;   /**< total number of received bytes */
    char *buffer;             /**< pointer to receiver buffer */
    char *buf;                /**< pointer to position within receiver buffer */
    char *backup_buf;         /** pointer to the buffer with partial messages if any **/
    int fd;                   /**< connection handle */
    DltReceiverType type;     /**< type of connection handle */
    uint32_t buffersize;      /**< size of receiver buffer */
    struct sockaddr_in addr;  /**< socket address information */
} DltReceiver;

while your version has swapped DltReceiverType and buffersize!

     typedef struct
    {
        int32_t lastBytesRcvd;    /**< bytes received in last receive call */
        int32_t bytesRcvd;        /**< received bytes */
        int32_t totalBytesRcvd;   /**< total number of received bytes */
        char *buffer;             /**< pointer to receiver buffer */
        char *buf;                /**< pointer to position within receiver buffer */
        char *backup_buf;         /** pointer to the buffer with partial messages if any **/
        int fd;                   /**< connection handle */
        int32_t buffersize;       /**< size of receiver buffer */
        DltReceiverType  type;    /**< type of connection handle */
        struct sockaddr_in addr;  /**< socket address information */
    } DltReceiver; 

[blublup]

@LocutusOfBorg indeed you're right !

Anyway the _fields_ is correct...

    _fields_ = [("lastBytesRcvd", ctypes.c_int32),
                ("bytesRcvd", ctypes.c_int32),
                ("totalBytesRcvd", ctypes.c_int32),
                ("buffer", ctypes.POINTER(ctypes.c_char)),
                ("buf", ctypes.POINTER(ctypes.c_char)),
                ("backup_buf", ctypes.POINTER(ctypes.c_char)),
                ("fd", ctypes.c_int),
                ("type", ctypes.c_int),
                ("buffersize", ctypes.c_int32),
                ("addr", sockaddr_in)]

[yen3]

I start to review @blublup 's PR. Hope we could process it smoothly.

[yen3]

@blublup @LocutusOfBorg Anything update for your patches?

[LocutusOfBorg]

Hello, I started from @blublup and added your suggestions, some of my changes, and something more, and pushed on a new PR that is the mix of the two already open. I will be AFK for a week or two, feel free to adapt, and push directly if you can

[aigarius]

Support up to 2.18.8 is now current.