-
-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
added 12mhz and CANopen support #84
Conversation
Baudrates calculation was done with: |
Hi @munzili ☕ 👋 Thank you for your PR. However, I'm not entirely happy with it, i.e. You've computed different values for baud-rates that are already known to be working, i.e. Before: static CanBitRateConfig constexpr BitRate_250kBPS_16MHz = {0x41, 0xF1, 0x85}; After: static CanBitRateConfig constexpr BitRate_250kBPS_16MHz = {0x01, 0xb5, 0x01}; That gives me little confidence in all the other values. Also I frankly don't have the time to manually check all the baud rates. I'm open to accept part of your changes but I'm not going to throw away baud-rates that I happen to know work - unless you take on the work of verifying all those baud rates on real hardware. Since 12 MHz are new and you seem to want that, why not focus on that first? |
Well, actually i need 12mhz support AND different baudrates. if you wish, i change the PR to only support the new baudrates and the 12mhz chip and leave the old values. I tested 12mhz with 20kbps. The new values should be conform with the CANopen standard based on this document https://www.can-cia.org/fileadmin/resources/documents/proceedings/2003_koppe.pdf by setting the baudrates and the sample point to the values defined in the document |
That would be an acceptable proposition. |
Btw. controller type in your online calculator seems to only support MCP2510. Not sure on the difference though. Here seems to be another tool: https://intrepidcs.com/products/free-tools/mb-time-calculator/ . Anything you can do to increase my confidence in this change would be appreciated. |
https://ww1.microchip.com/downloads/en/Appnotes/00872a.pdf the MCP2515 is based on MCP2510 but CNF flags are equal except CNF3 7bit but this bit was not setted before too |
Works for me 👍 . Only the changes for 12 MHz then plus your missing baud rates. |
Hi. I think with the previous settings we have a sample point of 0.75. |
So you'd offer to do some testing with these value @generationmake ? Relevant for our applications are 250k/500k/1MBit/s at 16 MHz. |
Memory usage change @ 5c0f116
Click for full report table
Click for full report CSV
|
I have restored the original flag definitions |
Memory usage change @ e059597
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thank you @munzili ☕ 👋
This pull requests adds