-
Notifications
You must be signed in to change notification settings - Fork 0
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
Check new CANDecoder implementation #1
base: main
Are you sure you want to change the base?
Conversation
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.
Good start! I think we can make the code a little less repetitive though...
The main difference between the first readHandler()
and sendSignal()
and the second pair of functions is just the values you send and the CAN IDs of the messages. Thus, we could just define 2 sets of macros at the top, then use those macros in the functions:
#if IS_FIRST_CAN
#define CAN_MSG_ID_BASE 0x000
#define FLOAT_VAL 3.2
#define UINT8_VAL 98
...
#else
#define CAN_MSG_ID_BASE 0x100
#define FLOAT_VAL 12.3
#define UINT8_VAL 45
...
#endif
void CANDecoder::readHandler(CAN_data msg) {
switch (msg.id) {
case (CAN_MSG_ID_BASE + 0):
...
int CANDecoder::sendSignal() {
uint8_t test8int = UINT8_VAL;
Some other tips:
- In general, I recommend compiling the code and trying to fix the errors before pushing any commits.
- Don't forget to resolve the comments after you've fixed them. It helps with the reviewing process!
Also, can you switch to the FW-221/canmanager branch in the |
…ion and comments from PR 'Check new CANDecoder Implementation'#1
This pull request is to notify you that I finished the first (untested) versions of the 'candecoder.h' and 'candecoder.cpp' files so far.
Please let me know if the code seems to be as per your guidelines or if there are any issues and thus changes that I need to make.