intrepidcs / python_ics

Library for interfacing with Intrepid devices in Python
MIT License
62 stars 31 forks source link

Inconsistent behavior of `get_messages` on Linux and Windows #84

Closed pierreluctg closed 4 years ago

pierreluctg commented 4 years ago

Hi, we are observing a very large delay between Tx and Rx (of that same Tx message) on Linux when using libicsneo legacy API.

I have attached a code example showing the issue. On windows this is results in a tx+rx delay mean time of about 0.0025 sec. on Linux we get about 1 sec.

This means that on Linux sending a message (CAN in our case) and receiving that same message on the same CAN adapter and same network takes about 1 seconds. This is, as you know a eternity.

The code attached basically send a message with the current time in the data portion, then receive that same message and compare the time stored in data (Tx time) and the current time (Rx time).

Can you please have a look.

Let me know if you need any additional information.

import struct
import time
from itertools import cycle
from statistics import mean, stdev

import ics

def tx(device):
    message = ics.SpyMessage()
    message.NetworkID = ics.NETID_HSCAN
    message.ArbIDOrHeader = 0x001
    message.NumberBytesData = 8
    ts = time.time()
    ba = bytearray(struct.pack("d", ts))
    message.Data = tuple(ba[:8])
    ics.transmit_messages(device, message)
    return ts

def rx(device):
    rx_message = None
    while rx_message is None:
        messages, errors = ics.get_messages(device, False, 1)
        for ics_msg in messages:
            is_tx = bool(ics_msg.StatusBitField & ics.SPY_STATUS_TX_MSG)
            if not is_tx:
                continue
            if bool(ics_msg.StatusBitField & ics.SPY_STATUS_GLOBAL_ERR):
                continue

            if ics_msg.ArbIDOrHeader == 0x001:
                rx_message = ics_msg

    return struct.unpack("d", bytearray(rx_message.Data))[0]

def tx_rx(device):
    tx_ts = tx(device)
    rx_ts = rx(device)
    tx_rx_time = time.time() - rx_ts

    # Confirm that we are looking at the right message
    assert tx_ts == rx_ts
    print(f"Tx and Rx took {tx_rx_time} seconds")
    return tx_rx_time

def main():
    devices = ics.find_devices()
    device = devices[0]
    print(device)
    ics.open_device(device)

    deltas = []
    for _ in range(1000):
        deltas.append(tx_rx(device))

    ics.close_device(device)

    print("=" * 20)
    print(f"Min: {min(deltas)}, Max: {max(deltas)}")
    print(f"Mean: {mean(deltas)}, Std: {stdev(deltas)}")

if __name__ == '__main__':
    main()
pierreluctg commented 4 years ago

Hi @drebbe-intrepid, please have a look.

I will also post a PR with a fix.

pierreluctg commented 4 years ago

Please see PR #85

drebbe-intrepid commented 4 years ago

This has been merged into v900 branch, needs to be pulled into other branches. I'll leave open for now with tags.

drebbe-intrepid commented 4 years ago

I still need to test this but I don't foresee any issues here.

drebbe-intrepid commented 4 years ago

PR #86

pierreluctg commented 4 years ago

Thank you @drebbe-intrepid. Can you also push to v901 and master branch?

Can we also have a new release of the v900 (4.x) branch with this fix? Or a first release from v901 (5.x) with this fix in?

drebbe-intrepid commented 4 years ago

@pierreluctg I'll push it to all new branches and I'll make a new release for v900 (4.x) branch. I just want to test this a bit more before releasing this one.

drebbe-intrepid commented 4 years ago

https://github.com/intrepidcs/python_ics/releases/tag/v4.6