General changes - CAN, light codes #2
Conversation
| @@ -70,7 +70,7 @@ | |||
| * quanta which means the bit rate is 8MHz / 16 = 500KHz. | |||
| */ | |||
| /* Nominal bit Baud Rate Prescaler */ | |||
| #define CONF_CAN_NBTP_NBRP_VALUE 5 | |||
| #define CONF_CAN_NBTP_NBRP_VALUE 1//This value has been changes from 5 to 1 | |||
There was a problem hiding this comment.
No need for this comment. Value of 5 was a bug, no need to be reminded of that ;)
| @@ -85,7 +85,7 @@ | |||
| * quanta which means the bit rate is 8MHz / 16 = 500KHz. | |||
| */ | |||
| /* Data bit Baud Rate Prescaler */ | |||
| #define CONF_CAN_DBTP_DBRP_VALUE 5 | |||
| #define CONF_CAN_DBTP_DBRP_VALUE 1//This value has been changes from 5 to 1 | |||
| delay_ms(1000); | ||
|
|
||
| uint8_t temp_pin = pin_num; | ||
| for(int i =0; i<4;i++){ |
There was a problem hiding this comment.
Try to keep format consistent with the rest of the codebase: equal spacing between operators, spacing between open brace and statement/function signature
| @@ -33,6 +33,10 @@ | |||
| // Function prototypes | |||
| uint8_t get_pinstrap_id(void); | |||
| inline float uint16ToC(uint16_t data); | |||
| void flash_pinstrap_id(uint8_t pin_num); | |||
There was a problem hiding this comment.
While there is some benefit in using the LED for these diagnostic/error reporting codes, I think it's too specific of a usage to be included in the base code. Let's maybe drop this commit from this pull request, but definitely good use of the LED for your purposes!
| @@ -49,6 +53,9 @@ uint8_t board_id = 255; | |||
| enum s2c_board_type board_type = S2C_BOARD_OTHER; | |||
| struct s2c_board_config board_config; | |||
|
|
|||
| bool FLASH_BOARD_ID = true; | |||
There was a problem hiding this comment.
In a resource constrained embedded system, every byte of flash or RAM is valuable. When statically configuring a behaviour, such as whether to flash the board ID or not, it's best to selectively include the code responsible for that feature, or to just not compile it in. You can do that with preprocessor macros. So:
#define FLASH_BOARD_ID TRUE
And then later, instead of having an if statement around the code that implements this feature, you would have a conditional macro:
#if (FLASH_BOARD_ID)
[code to flash board ID]
#endif
Now, if you need to turn this feature off, you simply set the macro to FALSE, and the whole code surrounded by the conditional macro will not get compiled in. This way, you don't waste any resources on unused functionality. Note that depending on how your compiler is configured, it might be smart enough to filter out the code the way you have it set up here, but using macros ensures that it's included or not with certainty.
Obviously the firmware in this repo is not at all at the flash or RAM limit of the microcontroller, but it's good to have this mentality when working with embedded systems at any scale, because as your code becomes more and more complex, you will be sure that you're using your limited resources wisely.
Change the configuration of the CAN packet to compensate for the 1/3 clock shift error discussed in issue #1. Also Added in 'flash codes' for board pin number and can status.
Flash codes:
Pin number: If the correct boolean is set to true (currently true) then on load up after board is configured it will blink rapidly(to get your attention) then do a binary flash of the number of the board ID. The least significant bit to most. This is not done asynchronously and will delay the board's recording by a couple of seconds. After this flash out the light will turn on to signify that the board is recording.
CAN status: If there is a CAN error it will rapidly blink the led. It is enabled by a separate boolean and is currently disabled.