stm32: Fix pyb.CAN BRS baud rate, make calculation more forgiving.#16544
Conversation
|
Code size report: |
projectgus
force-pushed
the
bugfix/stm32_canfd_brs
branch
from
dcb01e9 to
ef672a8
Compare
| mp_int_t baud_err = baudrate - calc_baud; | ||
| mp_int_t sample_err = sample_point - calc_sample; | ||
| if (abs(baud_err) < max_baud_error && | ||
| abs(sample_err) < MAX_SAMPLE_ERROR) { |
There was a problem hiding this comment.
What happens if there's an exact parameter match but it's not used because this algorithm finds a "close enough" match first? Maybe it needs to have a set of parameters "best match so far" and then it searches all options until it finds an exact match, or at the end falls back to a "close enough" match.
There was a problem hiding this comment.
I worried about this too, but I think the short answer is nothing noticeable. The error tolerances are pretty small: An unnecessary .1% baud rate error should still work fine unless the two nodes were already very marginal, and the chosen sample point should still match the argument when rounded to the nearest percent (the sample point argument doesn't support any higher precision than this).
(That said, the current version of the machine.CAN calculation does do the "track best option" you're describing, but that's mostly so that it can accept >.1% deviations rather than failing there as well. Although, after writing the simpler version here I was thinking about throwing that out to save code size. But I could add it to this PR instead, if you prefer. There's some small chance of regression though, as it's quite different.)
There was a problem hiding this comment.
Very Serious CAN users will likely always use the other configuration method (brp, bs1, bs2) anyway.
There was a problem hiding this comment.
Very Serious CAN users will likely always use the other configuration method (brp, bs1, bs2) anyway.
Yes that's a good point. So I guess it's fine to keep the code here as you have it, but maybe it's then worth documenting the fact that specifying baudrate will only get it to within 0.1% and if very precise timing is needed then it's better to specify bs1/bs2/brp.
There was a problem hiding this comment.
Great idea, have added docs for this change
projectgus
force-pushed
the
bugfix/stm32_canfd_brs
branch
4 times, most recently
from
bc62e17 to
8e5b573
Compare
Was initialising using the Classic CAN bs1/bs2 value, incorrectly. Signed-off-by: Angus Gratton <[email protected]>
Not every baudrate or sample point combination has an exact match, but getting within 1% on sample point and .1% on baud rate should always be good enough. Because the search goes from shorter bit periods (lowest brp) and increases, the first match which meets this criteria should still mostly be the best available. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <[email protected]>
This is redundant for bxCAN, but for CAN-FD with BRS it's otherwise unclear which set of parameters (baudrate & sample_point or brs_baudrate & brs_sample_point) failed to match. This makes finding a valid combination extra annoying. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <[email protected]>
dpgeorge
force-pushed
the
bugfix/stm32_canfd_brs
branch
from
8e5b573 to
221a4ec
Compare
2 participants
Summary
Testing