Skip to content

Bit timing config for PCAN #625

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Bit timing config for PCAN #625

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e8e328f
Bit timing config for PCAN
christiansandberg Jun 16, 2019
3dda5f1
Add debug for FD bit rate string
christiansandberg Jun 16, 2019
3f1c99c
Fix Lint
christiansandberg Jun 16, 2019
7894ebc
Merge branch 'develop' into pcan-bit-timing
felixdivo Jun 23, 2019
4f7ee78
Merge branch 'develop' into pcan-bit-timing
felixdivo Jun 28, 2019
931dd71
Merge branch 'develop' into pcan-bit-timing
felixdivo Jul 14, 2019
cf441e7
Merge branch 'develop' into pcan-bit-timing
felixdivo Jul 27, 2019
22dc9e3
Merge branch 'develop' into pcan-bit-timing
christiansandberg Nov 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 50 additions & 94 deletions can/interfaces/pcan/pcan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import logging
import time
import warnings

from typing import Optional
from can import CanError, Message, BusABC
Expand Down Expand Up @@ -62,26 +63,17 @@
}


pcan_fd_parameter_list = [
"nom_brp",
"nom_tseg1",
"nom_tseg2",
"nom_sjw",
"data_brp",
"data_tseg1",
"data_tseg2",
"data_sjw",
]


class PcanBus(BusABC):
def __init__(
self,
channel="PCAN_USBBUS1",
state=BusState.ACTIVE,
bitrate=500000,
timing=None,
data_timing=None,
fd=False,
*args,
**kwargs
**kwargs,
):
"""A PCAN USB interface to CAN.

Expand All @@ -104,77 +96,14 @@ def __init__(

:param bool fd:
Should the Bus be initialized in CAN-FD mode.

:param int f_clock:
Clock rate in Hz.
Any of the following:
20000000, 24000000, 30000000, 40000000, 60000000, 80000000.
Ignored if not using CAN-FD.
Pass either f_clock or f_clock_mhz.

:param int f_clock_mhz:
Clock rate in MHz.
Any of the following:
20, 24, 30, 40, 60, 80.
Ignored if not using CAN-FD.
Pass either f_clock or f_clock_mhz.

:param int nom_brp:
Clock prescaler for nominal time quantum.
In the range (1..1024)
Ignored if not using CAN-FD.

:param int nom_tseg1:
Time segment 1 for nominal bit rate,
that is, the number of quanta from (but not including)
the Sync Segment to the sampling point.
In the range (1..256).
Ignored if not using CAN-FD.

:param int nom_tseg2:
Time segment 2 for nominal bit rate,
that is, the number of quanta from the sampling
point to the end of the bit.
In the range (1..128).
Ignored if not using CAN-FD.

:param int nom_sjw:
Synchronization Jump Width for nominal bit rate.
Decides the maximum number of time quanta
that the controller can resynchronize every bit.
In the range (1..128).
Ignored if not using CAN-FD.

:param int data_brp:
Clock prescaler for fast data time quantum.
In the range (1..1024)
Ignored if not using CAN-FD.

:param int data_tseg1:
Time segment 1 for fast data bit rate,
that is, the number of quanta from (but not including)
the Sync Segment to the sampling point.
In the range (1..32).
Ignored if not using CAN-FD.

:param int data_tseg2:
Time segment 2 for fast data bit rate,
that is, the number of quanta from the sampling
point to the end of the bit.
In the range (1..16).
Ignored if not using CAN-FD.

:param int data_sjw:
Synchronization Jump Width for fast data bit rate.
Decides the maximum number of time quanta
that the controller can resynchronize every bit.
In the range (1..16).
Ignored if not using CAN-FD.

:param can.BitTiming timing:
Bit timing configuration.
For CAN-FD this also applies to arbitration/nominal phase.
:param can.BitTiming data_timing:
Bit timing configuration for data phase.
"""
self.channel_info = channel
self.fd = kwargs.get("fd", False)
pcan_bitrate = pcan_bitrate_objs.get(bitrate, PCAN_BAUD_500K)
self.fd = fd

hwtype = PCAN_TYPE_ISA
ioport = 0x02A0
Expand All @@ -189,24 +118,51 @@ def __init__(
raise ArgumentError("BusState must be Active or Passive")

if self.fd:
f_clock_val = kwargs.get("f_clock", None)
if f_clock_val is None:
f_clock = "{}={}".format("f_clock_mhz", kwargs.get("f_clock_mhz", None))
params = {}
if timing and data_timing:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a single timing object which includes both arbitration and data timings.
This also solves the problem of multiple f_clock settings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean that the class should handle both arbitration and data timings I'm not sure I agree about that. Apart from f_clock (which is not always required), everything else would have to be duplicated or require some overly complex code structures in that class. There would also need to be separate classes for CAN-FD and regular CAN to not add confusion for most users which do not use CAN-FD.

This solution does not need to consider if the timing settings are used for normal CAN, arbitration phase or data phase since they are basically the same and have no dependency apart from clock frequency and can therefore be specified separately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why this will complicate the usage as long as the data phase timings are optional.

Just to make sure we share the same understanding. Normal timings aka nom are used for both arbitration and data in standard CAN. Data timings are only used in CAN-FD for the data phase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For standard CAN there is no nominal or arbitration or data timing, just timing. I don’t think it makes sense to force CAN-FD nomenclature on standard CAN users, especially when the interfaces don’t even support CAN-FD.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree. The data timing are only used when the bit rate switch flag is marked. This means that the nominal bit timings have generally the same meaning as in classic CAN (and technically can be used with classic CAN on the same bus as long as the CAN ICs are FD bus compatible, even if they don't support the FD extensions). The data timings are the addition. The Peak software tools seem to treat the timings as I have described.

I don't see how this is forcing anything on classic CAN users. If you think the naming is clearer the way it is now I would add the data timings and not add the nominal prefix to the existing timings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this? Since timings are then grouped by prefix there is no added value for the user to have a separate class for specifying timing values. They can enter them into the Bus constructor directly. Internally we can construct BitTiming instances however using some utility function taking a bunch of kwargs and returning a nominal and data timing objects.

params["f_clock"] = timing.f_clock
params["nom_brp"] = timing.brp
params["nom_tseg1"] = timing.tseg1
params["nom_tseg2"] = timing.tseg2
params["nom_sjw"] = timing.sjw
params["data_brp"] = data_timing.brp
params["data_tseg1"] = data_timing.tseg1
params["data_tseg2"] = data_timing.tseg2
params["data_sjw"] = data_timing.sjw
elif "nom_tseg1" in kwargs:
warnings.warn(
"Specifying bit timing as direct keyword arguments is depreceated. Use the can.BitTiming class instead.",
DeprecationWarning,
)
if "f_clock" in kwargs:
params["f_clock"] = kwargs["f_clock"]
if "f_clock_mhz" in kwargs:
params["f_clock_mhz"] = kwargs["f_clock_mhz"]
params["nom_brp"] = kwargs["nom_brp"]
params["nom_tseg1"] = kwargs["nom_tseg1"]
params["nom_tseg2"] = kwargs["nom_tseg2"]
params["nom_sjw"] = kwargs["nom_sjw"]
params["data_brp"] = kwargs["data_brp"]
params["data_tseg1"] = kwargs["data_tseg1"]
params["data_tseg2"] = kwargs["data_tseg2"]
params["data_sjw"] = kwargs["data_sjw"]
else:
f_clock = "{}={}".format("f_clock", kwargs.get("f_clock", None))

fd_parameters_values = [f_clock] + [
"{}={}".format(key, kwargs.get(key, None))
for key in pcan_fd_parameter_list
if kwargs.get(key, None) is not None
]
raise ArgumentError("timing and data_timing arguments missing")

self.fd_bitrate = " ,".join(fd_parameters_values).encode("ascii")
fd_bitrate = ",".join(f"{key}={val}" for key, val in params.items())
log.debug("FD bit rate string: %s", fd_bitrate)

result = self.m_objPCANBasic.InitializeFD(
self.m_PcanHandle, self.fd_bitrate
self.m_PcanHandle, fd_bitrate.encode("ascii")
)
else:
if timing:
pcan_bitrate = TPCANBaudrate(timing.btr0 << 8 | timing.btr1)
elif bitrate in pcan_bitrate_objs:
pcan_bitrate = pcan_bitrate_objs[bitrate]
else:
log.warning("Unknown bitrate. Falling back to 500 kbit/s.")
pcan_bitrate = PCAN_BAUD_500K
result = self.m_objPCANBasic.Initialize(
self.m_PcanHandle, pcan_bitrate, hwtype, ioport, interrupt
)
Expand Down