Repository navigation
fix(bit-timing): support extended hardware limits - #2094
timothyanderson096-ocdealcheck wants to merge 3 commits into
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8865a0a935
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
63a1028 to
d039d48
Compare
|
|
||
| if not 1 <= self.tseg1 <= 16: | ||
| raise ValueError(f"tseg1 (={self.tseg1}) must be in [1...16].") | ||
| if not 1 <= self.tseg1 <= tseg1_max: |
There was a problem hiding this comment.
How can this ever fail if you set self._tseg1_max = max(16, tseg1) in the init method? Am i missing something?
There was a problem hiding this comment.
tseg1_max in this check is the _validate() argument (default 16), rather than self._tseg1_max. The stored attributes retain limits for clock recreation; the sample-point factory replaces those fallback values with the caller's explicit controller limits. For example, constructing with tseg1=17, strict=True still raises ValueError, even though the stored fallback maximum is 17. I've added a regression for that case and clarified the comment.
While checking this, I found a separate gap in the clock-recreation shortcut: a timing created at 8 MHz / 500 kbit/s with brp_max=1 could be recreated at 16 MHz with brp=2. In 33a212a, the shortcut validates against the retained controller limits before returning, then falls back to the bounded solver when necessary. The new regression rejects that case and accepts the brp_max=2 boundary control.
Local validation on Python 3.14.10: 36 timing/util tests passed; configured lint and all five mypy targets (3.11–3.15) passed; Sphinx HTML and 39 doctests passed. Black used one worker for this environment. The three intentional internal-access annotations follow the existing convention in this file. Hosted CI is running on the new revision.
Timothy Anderson — Spotted Ridge Engineering / OC Labs · GDN (Global Decision Network)
timothyanderson096@gmail.com
Apply the caller's retained controller bounds before accepting the prescaler shortcut. Fall back to the bounded solver when the shortcut exceeds a limit. Add rejection and boundary controls, clarify the independent validation limits, and annotate intentional internal access using the existing convention. Signed-off-by: TIM ANDERSON <timothyanderson096@gmail.com>
Summary of Changes
BitTiming.iterate_from_sample_point()andBitTiming.from_sample_point()to use controller-specifictseg1_max,tseg2_max, andbrp_maxlimits.At 160 MHz / 250 kbit/s / 87.5%, the default segment limits now allow
brp=40, tseg1=13, tseg2=2. Explicit extended limits of256/128/512allowbrp=4, tseg1=139, tseg2=20. Abrp_max=28control rejects the configuration when no candidate fits the default segment limits.Clock recreation also respects smaller controller limits: an 8 MHz / 500 kbit/s timing with
brp_max=1cannot be recreated at 16 MHz by returningbrp=2; the same change is accepted whenbrp_max=2. Strict construction withtseg1=17still raises independently of the stored fallback limits.Related Issues / Pull Requests
BitTiming.from_sample_pointrejects valid timing solutions due to hardcoded register limits #2083Type of Change
Checklist
Testing
Revision
33a212a86f94682b9ca740913e7163134f5e4efc, locally on Python 3.14.10:tox -e lint: Black, Ruff and Pylint pass. Black ran with one worker because this environment restricts multiprocessing.tox -e type: all configured mypy targets, Python 3.11–3.15, pass.tox -e docs: Sphinx HTML with warnings as errors succeeds; 39 doctests pass.Additional Notes
No physical CAN controller was tested locally. The change was prepared with Codex assistance; the local results above were run against the published source tree.
Timothy Anderson — Spotted Ridge Engineering / OC Labs · GDN (Global Decision Network)
timothyanderson096@gmail.com