Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check compatibility with python-can 4.0.0 #296

Closed
acolomb opened this issue Feb 19, 2022 · 9 comments
Closed

Check compatibility with python-can 4.0.0 #296

acolomb opened this issue Feb 19, 2022 · 9 comments

Comments

@acolomb
Copy link
Collaborator

acolomb commented Feb 19, 2022

We should make sure there are no incompatible API changes regarding python-canopen's usage of the python-can package version 4.0.0 released today.

If there are, I suggest to release a version 3.0.1 here which pins the requirement to the last python-can release 3.3.x.

Then when any incompatibilities are addressed, make a version 3.1.0 that depends on python-can >=4.0.0 of required or just keep >=3.0.0 and remove the limitation <4.0.0 again.

If we are already compatible after thorough testing, this issue can just be closed.

@christiansandberg
Copy link
Owner

I'm not aware of any changes that affects us directly, but I haven't tested anything. Hopefully the tests will catch any incompatibilities since some tests check the integration with python-can.

@VonSquiggles
Copy link

VonSquiggles commented Apr 21, 2022

I am seeing incompatibility with python-can version 4.0.0 using an ixxat device. between versions 3 and 4 they expanded their API to support different models of the ixxat tool (CAN FD vs non-FD), and their IXXATBus class now has a 'bus' attribute that is specific to the device model. Thus, my network object now has self.bus AND self.bus.bus, not just self.bus.

"""
Traceback (most recent call last):
File "C:...\CreatingAMenu.py", line 182, in
SendSyncMessage(0.1, 6)
File "C:...\CreatingAMenu.py", line 125, in SendSyncMessage
NETWORK.sync.start(sendInterval)
File "C:\Program Files\Python310\lib\site-packages\canopen\sync.py", line 38, in start
self._task = self.network.send_periodic(self.cob_id, [], self.period)
File "C:\Program Files\Python310\lib\site-packages\canopen\network.py", line 233, in send_periodic
return PeriodicMessageTask(can_id, data, period, self.bus, remote)
File "C:\Program Files\Python310\lib\site-packages\canopen\network.py", line 315, in init
self._start()
File "C:\Program Files\Python310\lib\site-packages\canopen\network.py", line 318, in _start
self._task = self.bus.send_periodic(self.msg, self.period)
File "C:\Program Files\Python310\lib\site-packages\can\bus.py", line 239, in send_periodic
periodic_tasks = self._periodic_tasks
AttributeError: 'IXXATBus' object has no attribute '_periodic_tasks'
"""

^ If I change line 233 in canopen\network.py to be "PeriodicMessageTask(can_id, data, period, self.bus.bus, remote)", I no longer get an error, but my sync message sends at seemingly the fastest rate possible. I am less familiar with threads so will likely not be able to tackle this myself.

@christiansandberg
Copy link
Owner

This is a bug in python-can. The IXXAT interface does not call the base class init method.

@VonSquiggles
Copy link

@christiansandberg The interface calls another class depending on the driver, which then calls the base class init method. It's not a bug - just an API change.

@christiansandberg
Copy link
Owner

If you try to use the send_periodic method outside of CANopen you will probably find that it doesn't work there either. The bus inside a bus is implementation specific to IXXAT and should be transparent to the API consumer.

@VonSquiggles
Copy link

You're right. Looks like hardbyte/python-can#1285 calls out that specific issue.

It should be made known to user that v4.0.0 is incompatible, even if it is due to known bugs on python-can side.

@VonSquiggles
Copy link

Opened pull request to remove python-can >=v4.0.0 as dependency option

@christiansandberg
Copy link
Owner

It's complicated since the bug is limited to a specific interface. Most users will not be affected and forcing a downgrade for everyone else may cause bigger issues. We still allow python-can 3.x so IXXAT users could still downgrade python-can until the issue is fixed. Maybe add a note to the README?

@fs570714
Copy link

fs570714 commented Jul 6, 2023

I already answered here #318 just for those who are looking for a solution to their problem with the IXXAT, this issue is solved with the pull request hardbyte/python-can#1532 of the python-can library.

@acolomb acolomb closed this as completed Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants