-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Add back CANNODE_NODE_ID for setting a static node ID #25669
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
base: main
Are you sure you want to change the base?
Conversation
int32_t cannode_node_id = 0; | ||
param_get(param_find("CANNODE_NODE_ID"), &cannode_node_id); | ||
|
||
if (node_id == 0 && cannode_node_id > 0 && cannode_node_id <= 127) { |
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.
At a minimum we should probably print an error if the node ID value isn't valid.
cannode_node_id = 0; | ||
} | ||
|
||
// Assign the static node ID if no dynamic allocation is used. Do wen't override a valid node ID from the bootloader? |
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.
This setup will use the dynamic ID from the bootloader, do we want to override it?
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.
Yes I think if the user has specified a static node ID via CANNODE_NODE_ID and it does not match the NodeID from the shared memory then we need to override it, print a warning, and update the value in shared memory.
This PR adds back the CANNODE_NODE_ID param for setting a static node ID on cannodes. This will need careful testing as the last time I dove into this, ran into too many edge cases that could prevent a node from booting.