Skip to content
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

A default for oscillator frequency can be determined by looking at F_CPU #20

Open
leres opened this issue Jul 14, 2019 · 2 comments
Open

Comments

@leres
Copy link
Contributor

leres commented Jul 14, 2019

I'm using something similar to:

#if F_CPU == 20000000UL
#define MCP_SPEED MCP_20MHZ
#elif F_CPU == 16000000UL
#define MCP_SPEED MCP_16MHZ
#elif F_CPU == 8000000UL
#define MCP_SPEED MCP_8MHZ
#else
#error "Unsupported F_CPU value"
#endif
[...]
mcp2515.setBitrate(CAN_500KBPS, MCP_SPEED);

But it wouldn't be difficult to change the default speed from 16MHz to a speed based on the arduino clock. I'm happy to make the changes and issue a pull request if this is interesting.

Also you could free up some program memory by changing setBitrate() to conditionally compiling the canClock cases.

@autowp
Copy link
Owner

autowp commented Jul 15, 2019

Conditionally compiling sounds reasonable but breaks back compatibility. Maybe a new version?

About clocks. Same Arduino device may work with different clocked mcp's

@leres
Copy link
Contributor Author

leres commented Jul 15, 2019

What I was thinking that by default the single argument version of setBitrate() would assume canSpeed is the same as F_CPU:

--- a/mcp2515.cpp
+++ b/mcp2515.cpp
@@ -185,7 +185,18 @@ MCP2515::ERROR MCP2515::setMode(const CANCTRL_REQOP_MODE mode)
 
 MCP2515::ERROR MCP2515::setBitrate(const CAN_SPEED canSpeed)
 {
-    return setBitrate(canSpeed, MCP_16MHZ);
+       /* Assume F_CPU is the same as canClock by default */
+#if F_CPU == 20000000UL
+#define MCP2515_SPEED MCP_20MHZ
+#elif F_CPU == 16000000UL
+#define MCP2515_SPEED MCP_16MHZ
+#elif F_CPU == 8000000UL
+#define MCP2515_SPEED MCP_8MHZ
+#else
+#error "Unsupported F_CPU value"
+#endif
+
+    return setBitrate(canSpeed, MCP2515_SPEED);
 }
 
 MCP2515::ERROR MCP2515::setBitrate(const CAN_SPEED canSpeed, CAN_CLOCK canClock)=

I guess that would break any sketches that use a 16 MHz canclock and and a non-16 MHz arduino clock (F_CPU set to something other than the default 16 MHz).

I see your point about conditionally compiling the canClock cases.

How about providing a convenience define?

--- a/mcp2515.h
+++ b/mcp2515.h
@@ -169,6 +169,15 @@
 #define MCP_20MHz_33k3BPS_CFG2 (0xFF)
 #define MCP_20MHz_33k3BPS_CFG3 (0x87)
 
+/* Define MCP_SPEED as a canClock based on F_CPU */
+#if F_CPU == 20000000UL
+#define MCP_SPEED MCP_20MHZ
+#elif F_CPU == 16000000UL
+#define MCP_SPEED MCP_16MHZ
+#elif F_CPU == 8000000UL
+#define MCP_SPEED MCP_8MHZ
+#endif
+
 enum CAN_CLOCK {
     MCP_20MHZ,
     MCP_16MHZ,

Then the user could just use:

    setBitrate(canSpeed, MCP_SPEED);

when the MCP2515 and arduino use the same clock?

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

No branches or pull requests

2 participants