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.

Sign up for GitHub

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

General changes - CAN, light codes #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+70 −2
Open

General changes - CAN, light codes #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc5f25f
Change CAN configuration to offset for the the 1/3 clock error
1mozolacal Feb 1, 2021
90ed550
Added information flash out
1mozolacal Feb 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions s2c_sensor_module/src/config/conf_can.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this comment. Value of 5 was a bug, no need to be reminded of that ;)

/* Nominal bit (Re)Synchronization Jump Width */
#define CONF_CAN_NBTP_NSJW_VALUE 3
/* Nominal bit Time segment before sample point */
Expand All @@ -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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

/* Data bit (Re)Synchronization Jump Width */
#define CONF_CAN_DBTP_DSJW_VALUE 3
/* Data bit Time segment before sample point */
Expand Down
68 changes: 68 additions & 0 deletions s2c_sensor_module/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

void flash_low();
void flash_high();
void quick_blink();

void configure_adc(void);
void configure_can(void);
Expand All @@ -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;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

bool FLASH_CAN_ERROR = false;

// ASF driver instances
struct adc_module adc_instance;
struct can_module can_instance;
Expand Down Expand Up @@ -106,7 +113,61 @@ inline float uint16ToC(uint16_t data) {
return (float)data * 0.02 - 273.15;
}

/*
Does a long blink on the user led
Warning: 1000ms delay total
*/
void flash_high(){
port_pin_set_output_level(LED_USER_PIN, true);
delay_ms(600);
port_pin_set_output_level(LED_USER_PIN, false);
delay_ms(400);
}

/*
does a short blink on the user led
Warning: 500ms delay
*/
void flash_low(){
port_pin_set_output_level(LED_USER_PIN, true);
delay_ms(50);
port_pin_set_output_level(LED_USER_PIN, false);
delay_ms(450);
}

/*
quick blink
*/
void quick_blink(){
port_pin_toggle_output_level(LED_USER_PIN);
delay_ms(30);
port_pin_toggle_output_level(LED_USER_PIN);
delay_ms(30);
}

/*
flashes out the binary of the board ID
Warning long delay
*/
void flash_pinstrap_id(uint8_t pin_num){
int blink_amount = 5;
for(int i =0; i <blink_amount; i++){
quick_blink();
}
port_pin_set_output_level(LED_USER_PIN, false);
delay_ms(1000);

uint8_t temp_pin = pin_num;
for(int i =0; i<4;i++){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to keep format consistent with the rest of the codebase: equal spacing between operators, spacing between open brace and statement/function signature

if (temp_pin & 1) {
flash_high();
} else {
flash_low();
}
temp_pin = temp_pin >> 1;//shift over
}

}

// Configuration functions

Expand Down Expand Up @@ -337,6 +398,10 @@ int main (void)

system_interrupt_enable_global();

if(FLASH_BOARD_ID){
flash_pinstrap_id(board_id);
}

// Turn on generic LED to indicate that config is done
port_pin_set_output_level(LED_USER_PIN, true);

Expand Down Expand Up @@ -366,5 +431,8 @@ void CAN0_Handler(void)
if ((status & CAN_PROTOCOL_ERROR_ARBITRATION) || (status & CAN_PROTOCOL_ERROR_DATA)) {
can_clear_interrupt_status(&can_instance, CAN_PROTOCOL_ERROR_ARBITRATION | CAN_PROTOCOL_ERROR_DATA);
//printf("Protocol error, please double check the clock in two boards. \r\n\r\n");
if(FLASH_CAN_ERROR){
quick_blink();
}
}
}