From a3d96fa6921cfe428f67037633b19e29938befb2 Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Thu, 5 Oct 2023 13:15:04 +0300 Subject: [PATCH 1/7] add the possibility to return all the possible solutions using the from_sample_point static methods --- can/bit_timing.py | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/can/bit_timing.py b/can/bit_timing.py index 285b1c302..238075eaf 100644 --- a/can/bit_timing.py +++ b/can/bit_timing.py @@ -1,6 +1,6 @@ # pylint: disable=too-many-lines import math -from typing import Iterator, List, Mapping, cast +from typing import Iterator, List, Mapping, Union, cast from can.typechecking import BitTimingDict, BitTimingFdDict @@ -213,9 +213,9 @@ def from_registers( @classmethod def from_sample_point( - cls, f_clock: int, bitrate: int, sample_point: float = 69.0 - ) -> "BitTiming": - """Create a :class:`~can.BitTiming` instance for a sample point. + cls, f_clock: int, bitrate: int, sample_point: float = 69.0, all_solutions: bool = False, + ) -> Union["BitTiming", List["BitTiming"]]: + """Create an instance or list of :class:`~can.BitTiming` instances for a sample point. This function tries to find bit timings, which are close to the requested sample point. It does not take physical bus properties into account, so the @@ -230,6 +230,8 @@ def from_sample_point( Bitrate in bit/s. :param int sample_point: The sample point value in percent. + :param bool all_solutions: + Return all the possible solutions instead of just the preferred solution. :raises ValueError: if the arguments are invalid. """ @@ -279,7 +281,10 @@ def from_sample_point( ): possible_solutions.sort(key=key, reverse=reverse) - return possible_solutions[0] + if all_solutions: + return possible_solutions + else: + return possible_solutions[0] @property def f_clock(self) -> int: @@ -735,8 +740,9 @@ def from_sample_point( nom_sample_point: float, data_bitrate: int, data_sample_point: float, - ) -> "BitTimingFd": - """Create a :class:`~can.BitTimingFd` instance for a given nominal/data sample point pair. + all_solutions: bool = False, + ) -> Union["BitTimingFd", List["BitTimingFd"]]: + """Create an instance or list of :class:`~can.BitTimingFd` instances for a sample point. This function tries to find bit timings, which are close to the requested sample points. It does not take physical bus properties into account, so the @@ -755,6 +761,8 @@ def from_sample_point( Data bitrate in bit/s. :param int data_sample_point: The sample point value of the data phase in percent. + :param bool all_solutions: + Return all the possible solutions instead of just the preferred solution. :raises ValueError: if the arguments are invalid. """ @@ -824,12 +832,13 @@ def from_sample_point( if not possible_solutions: raise ValueError("No suitable bit timings found.") - # prefer using the same prescaler for arbitration and data phase - same_prescaler = list( - filter(lambda x: x.nom_brp == x.data_brp, possible_solutions) - ) - if same_prescaler: - possible_solutions = same_prescaler + if not all_solutions: + # prefer using the same prescaler for arbitration and data phase + same_prescaler = list( + filter(lambda x: x.nom_brp == x.data_brp, possible_solutions) + ) + if same_prescaler: + possible_solutions = same_prescaler # sort solutions for key, reverse in ( @@ -848,7 +857,10 @@ def from_sample_point( ): possible_solutions.sort(key=key, reverse=reverse) - return possible_solutions[0] + if all_solutions: + return possible_solutions + else: + return possible_solutions[0] @property def f_clock(self) -> int: From cf2a80de506a77aa3d4833bd70bb355c76db6cd9 Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Thu, 5 Oct 2023 10:18:46 +0000 Subject: [PATCH 2/7] Format code with black --- can/bit_timing.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/can/bit_timing.py b/can/bit_timing.py index 238075eaf..426015bc1 100644 --- a/can/bit_timing.py +++ b/can/bit_timing.py @@ -213,7 +213,11 @@ def from_registers( @classmethod def from_sample_point( - cls, f_clock: int, bitrate: int, sample_point: float = 69.0, all_solutions: bool = False, + cls, + f_clock: int, + bitrate: int, + sample_point: float = 69.0, + all_solutions: bool = False, ) -> Union["BitTiming", List["BitTiming"]]: """Create an instance or list of :class:`~can.BitTiming` instances for a sample point. From 46325477990b4e2654f3cfcd6f925faf7c2c0e57 Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Mon, 9 Oct 2023 10:30:57 +0300 Subject: [PATCH 3/7] add iterators for timings from sample point --- can/bit_timing.py | 169 +++++++++++++++++++++++++++------------------- 1 file changed, 98 insertions(+), 71 deletions(-) diff --git a/can/bit_timing.py b/can/bit_timing.py index 238075eaf..cebb9131c 100644 --- a/can/bit_timing.py +++ b/can/bit_timing.py @@ -212,34 +212,12 @@ def from_registers( ) @classmethod - def from_sample_point( - cls, f_clock: int, bitrate: int, sample_point: float = 69.0, all_solutions: bool = False, - ) -> Union["BitTiming", List["BitTiming"]]: - """Create an instance or list of :class:`~can.BitTiming` instances for a sample point. - - This function tries to find bit timings, which are close to the requested - sample point. It does not take physical bus properties into account, so the - calculated bus timings might not work properly for you. - - The :func:`oscillator_tolerance` function might be helpful to evaluate the - bus timings. - - :param int f_clock: - The CAN system clock frequency in Hz. - :param int bitrate: - Bitrate in bit/s. - :param int sample_point: - The sample point value in percent. - :param bool all_solutions: - Return all the possible solutions instead of just the preferred solution. - :raises ValueError: - if the arguments are invalid. - """ - + def iterate_from_sample_point( + cls, f_clock: int, bitrate: int, sample_point: float = 69.0 + ) -> Iterator["BitTiming"]: if sample_point < 50.0: raise ValueError(f"sample_point (={sample_point}) must not be below 50%.") - possible_solutions: List[BitTiming] = [] for brp in range(1, 65): nbt = round(int(f_clock / (bitrate * brp))) if nbt < 8: @@ -265,10 +243,41 @@ def from_sample_point( sjw=sjw, strict=True, ) - possible_solutions.append(bt) + yield bt except ValueError: continue + @classmethod + def from_sample_point( + cls, f_clock: int, bitrate: int, sample_point: float = 69.0 + ) -> "BitTiming": + """Create a :class:`~can.BitTiming` instance for a sample point. + + This function tries to find bit timings, which are close to the requested + sample point. It does not take physical bus properties into account, so the + calculated bus timings might not work properly for you. + + The :func:`oscillator_tolerance` function might be helpful to evaluate the + bus timings. + + :param int f_clock: + The CAN system clock frequency in Hz. + :param int bitrate: + Bitrate in bit/s. + :param int sample_point: + The sample point value in percent. + :raises ValueError: + if the arguments are invalid. + """ + + if sample_point < 50.0: + raise ValueError(f"sample_point (={sample_point}) must not be below 50%.") + + possible_solutions: List[BitTiming] = [ + timing + for timing in cls.iterate_from_sample_point(f_clock, bitrate, sample_point) + ] + if not possible_solutions: raise ValueError("No suitable bit timings found.") @@ -281,10 +290,7 @@ def from_sample_point( ): possible_solutions.sort(key=key, reverse=reverse) - if all_solutions: - return possible_solutions - else: - return possible_solutions[0] + return possible_solutions[0] @property def f_clock(self) -> int: @@ -733,39 +739,14 @@ def from_bitrate_and_segments( # pylint: disable=too-many-arguments return bt @classmethod - def from_sample_point( + def iterate_from_sample_point( cls, f_clock: int, nom_bitrate: int, nom_sample_point: float, data_bitrate: int, data_sample_point: float, - all_solutions: bool = False, - ) -> Union["BitTimingFd", List["BitTimingFd"]]: - """Create an instance or list of :class:`~can.BitTimingFd` instances for a sample point. - - This function tries to find bit timings, which are close to the requested - sample points. It does not take physical bus properties into account, so the - calculated bus timings might not work properly for you. - - The :func:`oscillator_tolerance` function might be helpful to evaluate the - bus timings. - - :param int f_clock: - The CAN system clock frequency in Hz. - :param int nom_bitrate: - Nominal bitrate in bit/s. - :param int nom_sample_point: - The sample point value of the arbitration phase in percent. - :param int data_bitrate: - Data bitrate in bit/s. - :param int data_sample_point: - The sample point value of the data phase in percent. - :param bool all_solutions: - Return all the possible solutions instead of just the preferred solution. - :raises ValueError: - if the arguments are invalid. - """ + ) -> Iterator["BitTimingFd"]: if nom_sample_point < 50.0: raise ValueError( f"nom_sample_point (={nom_sample_point}) must not be below 50%." @@ -776,8 +757,6 @@ def from_sample_point( f"data_sample_point (={data_sample_point}) must not be below 50%." ) - possible_solutions: List[BitTimingFd] = [] - sync_seg = 1 for nom_brp in range(1, 257): @@ -825,20 +804,71 @@ def from_sample_point( data_sjw=data_sjw, strict=True, ) - possible_solutions.append(bt) + yield bt except ValueError: continue + @classmethod + def from_sample_point( + cls, + f_clock: int, + nom_bitrate: int, + nom_sample_point: float, + data_bitrate: int, + data_sample_point: float, + ) -> "BitTimingFd": + """Create a :class:`~can.BitTimingFd` instance for a sample point. + + This function tries to find bit timings, which are close to the requested + sample points. It does not take physical bus properties into account, so the + calculated bus timings might not work properly for you. + + The :func:`oscillator_tolerance` function might be helpful to evaluate the + bus timings. + + :param int f_clock: + The CAN system clock frequency in Hz. + :param int nom_bitrate: + Nominal bitrate in bit/s. + :param int nom_sample_point: + The sample point value of the arbitration phase in percent. + :param int data_bitrate: + Data bitrate in bit/s. + :param int data_sample_point: + The sample point value of the data phase in percent. + :raises ValueError: + if the arguments are invalid. + """ + if nom_sample_point < 50.0: + raise ValueError( + f"nom_sample_point (={nom_sample_point}) must not be below 50%." + ) + + if data_sample_point < 50.0: + raise ValueError( + f"data_sample_point (={data_sample_point}) must not be below 50%." + ) + + possible_solutions: List[BitTimingFd] = [ + timing + for timing in cls.iterate_from_sample_point( + f_clock, + nom_bitrate, + nom_sample_point, + data_bitrate, + data_sample_point, + ) + ] + if not possible_solutions: raise ValueError("No suitable bit timings found.") - if not all_solutions: - # prefer using the same prescaler for arbitration and data phase - same_prescaler = list( - filter(lambda x: x.nom_brp == x.data_brp, possible_solutions) - ) - if same_prescaler: - possible_solutions = same_prescaler + # prefer using the same prescaler for arbitration and data phase + same_prescaler = list( + filter(lambda x: x.nom_brp == x.data_brp, possible_solutions) + ) + if same_prescaler: + possible_solutions = same_prescaler # sort solutions for key, reverse in ( @@ -857,10 +887,7 @@ def from_sample_point( ): possible_solutions.sort(key=key, reverse=reverse) - if all_solutions: - return possible_solutions - else: - return possible_solutions[0] + return possible_solutions[0] @property def f_clock(self) -> int: From 8b224e446e7d703b8615dcd600e649bed7bcaad0 Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Mon, 9 Oct 2023 10:38:30 +0300 Subject: [PATCH 4/7] update docstrings --- can/bit_timing.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/can/bit_timing.py b/can/bit_timing.py index ea5285eea..dc5877c68 100644 --- a/can/bit_timing.py +++ b/can/bit_timing.py @@ -216,6 +216,18 @@ def from_registers( def iterate_from_sample_point( cls, f_clock: int, bitrate: int, sample_point: float = 69.0 ) -> Iterator["BitTiming"]: + """Create a :class:`~can.BitTiming` iterator with all the solutions for a sample point. + + :param int f_clock: + The CAN system clock frequency in Hz. + :param int bitrate: + Bitrate in bit/s. + :param int sample_point: + The sample point value in percent. + :raises ValueError: + if the arguments are invalid. + """ + if sample_point < 50.0: raise ValueError(f"sample_point (={sample_point}) must not be below 50%.") @@ -748,6 +760,21 @@ def iterate_from_sample_point( data_bitrate: int, data_sample_point: float, ) -> Iterator["BitTimingFd"]: + """Create an :class:`~can.BitTimingFd` iterator with all the solutions for a sample point. + + :param int f_clock: + The CAN system clock frequency in Hz. + :param int nom_bitrate: + Nominal bitrate in bit/s. + :param int nom_sample_point: + The sample point value of the arbitration phase in percent. + :param int data_bitrate: + Data bitrate in bit/s. + :param int data_sample_point: + The sample point value of the data phase in percent. + :raises ValueError: + if the arguments are invalid. + """ if nom_sample_point < 50.0: raise ValueError( f"nom_sample_point (={nom_sample_point}) must not be below 50%." From f769ec0de6544c1fcb30e3413710452bfab7403b Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Mon, 9 Oct 2023 10:41:41 +0300 Subject: [PATCH 5/7] make ruff happy --- can/bit_timing.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/can/bit_timing.py b/can/bit_timing.py index dc5877c68..f5d50eac1 100644 --- a/can/bit_timing.py +++ b/can/bit_timing.py @@ -286,10 +286,9 @@ def from_sample_point( if sample_point < 50.0: raise ValueError(f"sample_point (={sample_point}) must not be below 50%.") - possible_solutions: List[BitTiming] = [ - timing - for timing in cls.iterate_from_sample_point(f_clock, bitrate, sample_point) - ] + possible_solutions: List[BitTiming] = list( + cls.iterate_from_sample_point(f_clock, bitrate, sample_point) + ) if not possible_solutions: raise ValueError("No suitable bit timings found.") @@ -877,16 +876,15 @@ def from_sample_point( f"data_sample_point (={data_sample_point}) must not be below 50%." ) - possible_solutions: List[BitTimingFd] = [ - timing - for timing in cls.iterate_from_sample_point( + possible_solutions: List[BitTimingFd] = list( + cls.iterate_from_sample_point( f_clock, nom_bitrate, nom_sample_point, data_bitrate, data_sample_point, ) - ] + ) if not possible_solutions: raise ValueError("No suitable bit timings found.") From 511563491af7b8ff2f92bdb41a146ea7f9c94041 Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Thu, 12 Oct 2023 14:12:34 +0300 Subject: [PATCH 6/7] add a small test for iterate_from_sample_point --- test/test_bit_timing.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/test_bit_timing.py b/test/test_bit_timing.py index 6852687a5..c3927744f 100644 --- a/test/test_bit_timing.py +++ b/test/test_bit_timing.py @@ -286,6 +286,28 @@ def test_from_sample_point(): ) +def test_iterate_from_sample_point(): + for sp in range(50, 100): + solutions = list(can.BitTiming.iterate_from_sample_point( + f_clock=16_000_000, + bitrate=500_000, + sample_point=sp, + )) + assert len(solutions) >= 2 + + for nsp in range(50, 100): + for dsp in range(50, 100): + solutions = list(can.BitTimingFd.iterate_from_sample_point( + f_clock=80_000_000, + nom_bitrate=500_000, + nom_sample_point=nsp, + data_bitrate=2_000_000, + data_sample_point=dsp, + )) + + assert len(solutions) >= 2 + + def test_equality(): t1 = can.BitTiming.from_registers(f_clock=8_000_000, btr0=0x00, btr1=0x14) t2 = can.BitTiming(f_clock=8_000_000, brp=1, tseg1=5, tseg2=2, sjw=1, nof_samples=1) From 44874b46265c2796112d8aa735e6a07ca0a76dbe Mon Sep 17 00:00:00 2001 From: danielhrisca Date: Thu, 12 Oct 2023 11:15:12 +0000 Subject: [PATCH 7/7] Format code with black --- test/test_bit_timing.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/test/test_bit_timing.py b/test/test_bit_timing.py index c3927744f..514c31244 100644 --- a/test/test_bit_timing.py +++ b/test/test_bit_timing.py @@ -288,22 +288,26 @@ def test_from_sample_point(): def test_iterate_from_sample_point(): for sp in range(50, 100): - solutions = list(can.BitTiming.iterate_from_sample_point( - f_clock=16_000_000, - bitrate=500_000, - sample_point=sp, - )) + solutions = list( + can.BitTiming.iterate_from_sample_point( + f_clock=16_000_000, + bitrate=500_000, + sample_point=sp, + ) + ) assert len(solutions) >= 2 for nsp in range(50, 100): for dsp in range(50, 100): - solutions = list(can.BitTimingFd.iterate_from_sample_point( - f_clock=80_000_000, - nom_bitrate=500_000, - nom_sample_point=nsp, - data_bitrate=2_000_000, - data_sample_point=dsp, - )) + solutions = list( + can.BitTimingFd.iterate_from_sample_point( + f_clock=80_000_000, + nom_bitrate=500_000, + nom_sample_point=nsp, + data_bitrate=2_000_000, + data_sample_point=dsp, + ) + ) assert len(solutions) >= 2