Skip to content

Conversation

@rnd-ash
Copy link
Contributor

@rnd-ash rnd-ash commented Jul 7, 2025

This PR resolves calculating prescaler when a fractional baud rate is requested (IE: 83333bps or 666666bps)

The prescaler function errors out if f_can/f_out*tq does not equal 0. This means for fractional baudrates, the MCAN's clock would also need to be fractional, which is very hard to achieve on most MCUs.

Therefore, this PR adjusts the prescaler function to allow for a maximum error of 0.05% (CAN has a tolerance of 0.75% on CAN FD and ~1% on normal CAN), rather than requiring a perfect clock combination.

@rnd-ash rnd-ash requested a review from a team as a code owner July 7, 2025 09:40
@Ironedde
Copy link
Contributor

Ironedde commented Sep 8, 2025

Hey @rnd-ash!

Sorry for not getting around to you about your suggestion about the bit rate, happy to see you added your own contribution!

I have not been exposed to fractional bit rates before when working on different CAN products. However, it is as you say that the CAN standard supports arbitrary rates, but was this mostly a thing back in the early days of CAN?

Nevertheless, I don't have any issue with adding this since it doesn't change how we calculate the prescaler. Maybe @epontan has something to add to this.

Also, I couldn't find the bit rate tolerance in ISO-11898-1. Do you know where this is specified?

@rnd-ash
Copy link
Contributor Author

rnd-ash commented Sep 8, 2025

Hey @Ironedde thanks for the reply,

Fractional bit-rates is something that Mercedes liked to use before 2010 for their vehicles (Fault-tolerant CAN at 83333bps).

I am aware it is quite uncommon, but it still is something the MCAN crate lacks support for.

Regarding the tolorance, I don't have access to the ISO specifications themselves, but a tolerance of 0.5%-1.58% is mentioned in some design guidelines from NXP and Microchip from years back when talking about oscillator frequency to drive CAN.

@epontan
Copy link
Collaborator

epontan commented Sep 8, 2025

Thank you for the contribution @rnd-ash!

I think in most cases one would want to get an error if the clock and bitrate are not perfectly matching. Would it make sense to the user to configure the tolerance in the BitTimings struct and default to 0 to for strictness?

@rnd-ash
Copy link
Contributor Author

rnd-ash commented Sep 8, 2025

I think it might be better to just allow for a "good as possible" or "perfect" setting. That way it avoids the user having to try and calculate the tolerance threshold for their own baudrate and clock setup. Obviously, if the baudrate is too far out from the requested, we should still return an error

@epontan
Copy link
Collaborator

epontan commented Sep 8, 2025

I'm OK with that too. The documentation of the bitrate field of the BitTimings struct needs to be updated to reflect this new tolerance though.

@rnd-ash
Copy link
Contributor Author

rnd-ash commented Sep 9, 2025

@epontan I've added a field to the BitTiming struct to allow for configuration if fractional bitrates are allowed or not, and also documented the tolerance.

epontan
epontan previously approved these changes Sep 9, 2025
Copy link
Collaborator

@epontan epontan left a comment

Choose a reason for hiding this comment

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

Nice work @rnd-ash! LGTM 👍

@rnd-ash
Copy link
Contributor Author

rnd-ash commented Sep 10, 2025

@epontan I've pushed a commit that should fix the pipelines. Can you run the pipelines again, they should pass now.

@Ironedde
Copy link
Contributor

Regarding the tolorance, I don't have access to the ISO specifications themselves, but a tolerance of 0.5%-1.58% is mentioned in some design guidelines from NXP and Microchip from years back when talking about oscillator frequency to drive CAN.

Alright, that's fine. We can always change the tolerance later since we're not making the fractional rate mandatory 👍

This looks fine to me too, just rebase your changes on top of 097f438 (current HEAD of master) and add your changes to the mcan/CHANGELOG.md under "Unreleased" (example #56 ).

Then we should be good to go 💯

@rnd-ash
Copy link
Contributor Author

rnd-ash commented Sep 11, 2025

@Ironedde it should be already rebased on that commit. I've updated the changelog

@Ironedde
Copy link
Contributor

Right, I see now. I'm use to running without merge commits, and therefore I got confused about the history of your master branch. My mistake.

LGTM, let's re-run the runners 👍

Copy link
Contributor

@Ironedde Ironedde left a comment

Choose a reason for hiding this comment

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

Approved, and thanks for your contribution 👍

@Ironedde Ironedde requested a review from epontan September 11, 2025 19:09
@epontan epontan merged commit 3cbdeed into GrepitAB:master Sep 12, 2025
5 checks passed
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

Successfully merging this pull request may close these issues.

3 participants