OpenCyphal / pycyphal

Python implementation of the Cyphal protocol stack.
https://pycyphal.readthedocs.io/
MIT License
119 stars 106 forks source link

feature: Please Add canalystii to pyuavcan/transport/can/media/pythoncan #178

Closed Cherish-forever closed 3 years ago

Cherish-forever commented 3 years ago

python-can support canalystii device, I think pyuavcan need support it

https://github.com/hardbyte/python-can/blob/develop/can/interfaces/canalystii.py

pavel-kirienko commented 3 years ago

Indeed, a pull request would be welcome.

Cherish-forever commented 3 years ago

@pavel-kirienko I have tried, but failed. I have add

def _construct_canalystii(parameters: _InterfaceParameters) -> can.ThreadSafeBus:
    if isinstance(parameters, _ClassicInterfaceParameters):
        return can.ThreadSafeBus(interface=parameters.interface_name, bitrate=parameters.bitrate)
    if isinstance(parameters, _FDInterfaceParameters):
        return can.ThreadSafeBus(
            interface=parameters.interface_name,
            channel=parameters.channel_name,
            bitrate=parameters.bitrate,
        )
    assert False, "Internal error"

and

        "canalystii": _construct_canalystii,

to pyuavcan/transport/can/media/pythoncan/_pythoncan.py

then

$ export YAKUT_TRANSPORT="CAN(can.media.pythoncan.PythonCANMedia(\"canalystii:0\", 1_000_000), 42)"

but failed:

$ yakut sub uavcan.node.Heartbeat.1.0
Usage: yakut subscribe [OPTIONS] [SUBJECT]...

Error: Invalid value: Could not initialize transport 'CAN(can.media.pythoncan.PythonCANMedia("canalystii:0", 1_000_000), 42)': TypeError("__init__() missing 1 required positional argument: 'channel'")

I don`t know why

pavel-kirienko commented 3 years ago

@Polarisru do you happen to have a suggestion here?

Cherish-forever commented 3 years ago

@pavel-kirienko @Polarisru I am using yakut and I have debug the yakut, it dead in: click/core.py(610)invoke()

pavel-kirienko commented 3 years ago

@Cherish-forever after you reformatted the source properly, it is obvious that you forgot to pass channel=parameters.channel_name, in the Classic case. Fix that and see if it works.

Usage of YAKUT_TRANSPORT is generally discouraged, consider using UAVCAN__CAN__IFACE instead as suggested in the README. This is not related to the current problem though, just a general suggestion.

Cherish-forever commented 3 years ago

@pavel-kirienko I have been run it pass.

here is my change:

def _construct_canalystii(parameters: _InterfaceParameters) -> can.ThreadSafeBus:
    if isinstance(parameters, _ClassicInterfaceParameters):
        return can.ThreadSafeBus(interface=parameters.interface_name, bitrate=parameters.bitrate)
    if isinstance(parameters, _FDInterfaceParameters):
        return can.ThreadSafeBus(
            interface=parameters.interface_name,
            channel=parameters.channel_name,
            bitrate=parameters.bitrate[0],
        )
    assert False, "Internal error"

and download canalystii so lib: https://github.com/AbangLZU/can_control/blob/master/can_control_node/lib/libcontrolcan.so

because: https://github.com/hardbyte/python-can/blob/develop/can/interfaces/canalystii.py#L65

so lib need put in current run path.

then you can use canalystii device.

oh here is my enverioment:

export YAKUT_COMPILE_OUTPUT=~/.yakut
export YAKUT_PATH="$YAKUT_COMPILE_OUTPUT"
export UAVCAN__NODE__ID=42
export UAVCAN__CAN__IFACE="canalystii:0"
export UAVCAN__CAN__MTU="8"
export UAVCAN__CAN__BITRATE=1000000
pavel-kirienko commented 3 years ago

The FD case discards the data segment bit rate, which is incorrect:

bitrate=parameters.bitrate[0]

Cherish-forever commented 3 years ago

@pavel-kirienko CANalyst-II MCU is PIC32MX795F512L ---- 2 x CAN2.0b modules with 1024 buffers here is the detail: https://www.microchip.com/en-us/product/PIC32MX795F512L

not support can fd.

pavel-kirienko commented 3 years ago

Then you should throw an error from the FD case instead of defaulting to the Classic mode.

Cherish-forever commented 3 years ago

@pavel-kirienko like this?

def _construct_canalystii(parameters: _InterfaceParameters) -> can.ThreadSafeBus:
    if isinstance(parameters, _ClassicInterfaceParameters):
        return can.ThreadSafeBus(interface=parameters.interface_name, bitrate=parameters.bitrate)
    if isinstance(parameters, _FDInterfaceParameters):
        return can.ThreadSafeBus(
            interface=parameters.interface_name,
            channel=parameters.channel_name,
            bitrate=parameters.bitrate[0],
        )
    if isinstance(parameters, _FDInterfaceParameters):
        raise InvalidMediaConfigurationError(f"Interface does not support CAN FD: {parameters.interface_name}")
    assert False, "Internal error"
pavel-kirienko commented 3 years ago

The second instance check is incorrect, otherwise yes.

Cherish-forever commented 3 years ago

@pavel-kirienko I have some confuse: why I have do:

export YAKUT_COMPILE_OUTPUT=~/.yakut
export YAKUT_PATH="$YAKUT_COMPILE_OUTPUT"
export UAVCAN__NODE__ID=42
export UAVCAN__CAN__IFACE="canalystii:0"
export UAVCAN__CAN__MTU="8"
export UAVCAN__CAN__BITRATE=1000000

and I have got instance is _FDInterfaceParameters.

in file: _transport_factory.py (274)

    br_arb, br_data = init("bitrate", Natural32([1_000_000, 4_000_000])).ints

here br_arb=1000000, and br_data=0, so in Line 288

media = PythonCANMedia(iface, br_arb if br_arb == br_data else (br_arb, br_data), mtu)

pass bitrate is (br_arb, br_data), this meas that device support CAN FD ?

pavel-kirienko commented 3 years ago

Can you rephrase the question, please?

pavel-kirienko commented 3 years ago

pass bitrate is (br_arb, br_data), this meas that device support CAN FD ?

It doesn't say anything about the capabilities of the device itself. It only means that if arbitration != data bitrate, the caller is expecting the device to support CAN FD.

Cherish-forever commented 3 years ago

@pavel-kirienko Thank you very much, I got it.

export UAVCAN__CAN__BITRATE="1000000 1000000"

and

def _construct_canalystii(parameters: _InterfaceParameters) -> can.ThreadSafeBus:
    if isinstance(parameters, _ClassicInterfaceParameters):
        return can.ThreadSafeBus(interface=parameters.interface_name,
                                 channel=parameters.channel_name,
                                 bitrate=parameters.bitrate)
    if isinstance(parameters, _FDInterfaceParameters):
        raise InvalidMediaConfigurationError(f"Interface does not support CAN FD: {parameters.interface_name}")
    assert False, "Internal error"
Cherish-Gww commented 3 years ago

@pavel-kirienko Hi, I have PR for this feature, and this is my first commit to open source project, and I don`t know how to add reviewers.

Cherish-Gww commented 3 years ago

pyuavcan base on python-can If you want use CANalyst-II, you need install python-can package. I have contact to CANalyst-II manufacturer, he does not provide he`s source code, so you need download CANalyst library or you can install python-can by this:

$ pip3 install git+https://github.com/Cherish-Gww/python-can.git@add_canalystii_so

it will automatic install libraries to your local. only test on x86_64 ubuntu

Linux xxxx 5.4.0-80-generic #90~18.04.1-Ubuntu SMP Tue Jul 13 19:40:02 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
pavel-kirienko commented 3 years ago

@Cherish-forever Consider sending a pull request adding these notes to the docs, maybe? This place is not discoverable at all for users, so very few people are going to see what you just wrote.

Cherish-Gww commented 3 years ago

@Cherish-forever Consider sending a pull request adding these notes to the docs, maybe? This place is not discoverable at all for users, so very few people are going to see what you just wrote.

like this?

            - Interface ``canalystii`` is described in
              https://python-can.readthedocs.io/en/stable/interfaces/canalystii.html.
              You need download CANalyst library for python-can package or you can install python-can by:
              ``pip3 install git+https://github.com/Cherish-Gww/python-can.git@add_canalystii_so``
              Example: ``canalystii:0``

in file pyuavcan/transport/can/media/pythoncan/_pythoncan.py Line: 89

pavel-kirienko commented 3 years ago

Yep, this seems sensible. Are you planning to have your fork merged into upstream of Python-CAN?

Cherish-Gww commented 3 years ago

Yep, this seems sensible. Are you planning to have your fork merged into upstream of Python-CAN?

https://github.com/hardbyte/python-can/pull/1123 they don`t want add the binary DLLs/SOs to python-can.

MKS-Howard commented 2 years ago

The DLL in your library is 64-bit. If you use python-32bit, ControlCAN.dll must be changed to 32-bit or python-64bit.

Cherish-Gww commented 2 years ago

Good news:

This driver is based on black box reverse engineering of the USB behaviour of the proprietary software, and reading the basic data structure layouts in the original python-can canalystii source.

https://github.com/hardbyte/python-can/commit/18f346211bf937593fbe2304d3f850be2474a129 hosted at https://github.com/projectgus/python-canalystii

python-can 4.0.0 will solve this issue. but not now.