Fwxv 250 update drive fsm #240
Conversation
libraries/master/inc/master_task.h
Outdated
| @@ -2,6 +2,8 @@ | |||
| #include "log.h" | |||
| #include "tasks.h" | |||
|
|
|||
| DECLARE_TASK(master_task); | |||
There was a problem hiding this comment.
Added so that we can reference "master_task" when sending notifications, etc
| @@ -0,0 +1,191 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
Initial commit of exported enums. Some are from XV, the rest are leftovers from MSXIV. Expect this to change
| #define BEGIN_PRECHARGE_SIGNAL 2 | ||
| #define PRECHARGE_STATE_COMPLETE 2 | ||
| #define NUMBER_OF_CYCLES_TO_WAIT 10 | ||
| #define POWER_FSM_STATE_OFF 0 // will be changed later |
There was a problem hiding this comment.
These need to go in exported enums
|
|
||
| if (pca9555_reg_val == PCA9555_REG_DEFAULT) { // No button pressed | ||
| return STATUS_CODE_OK; | ||
| } |
There was a problem hiding this comment.
We could add this to the pca interface, might make a good new member ticket
|
@JarvisWeng Main changes to this are moving all of the hardware interaction for Centre Console to one file, update_dashboard.c. Would be a good place to start your review. You can also look at other places where I've added comments. Ayo is working on validating these changes, but might be better to merge them in so we don't block others |
| #define CC_LED_PUSH_START \ | ||
| { .port = NUM_GPIO_PORTS, .pin = GPIO_PINS_PER_PORT } | ||
| #define CC_LED_POWER \ | ||
| { .i2c_address = CC_IO_EXP_ADDR, .pin = PCA9555_PIN_IO0_4 } |
There was a problem hiding this comment.
Ayo going to change all these addresses right?
|
|
||
| if (pca9555_reg_val == PCA9555_REG_DEFAULT) { // No button pressed | ||
| return STATUS_CODE_OK; | ||
| } | ||
| if ((~(pca9555_reg_val)®EN_BTN_MASK) != 0) { |
There was a problem hiding this comment.
Right before this if, maybe do pca9555_reg_val = ~pca9555_reg_val. That way you don't do it for every if. Or maybe the compiler optimizes this out anyway.
| prv_speed_is_negative()) { | ||
| next_state = REVERSE; | ||
| fsm_transition(fsm, DRIVE); | ||
| break; |
There was a problem hiding this comment.
Maybe just do a return here?
You do this to only wait on STATUS_CODE_INCOMPLETE? So you only want the first notification?
| // this is a fail case | going back to neutral | ||
| fsm_shared_mem_set_drive_error_code(STATUS_CODE_TIMEOUT); | ||
| next_state = NEUTRAL; | ||
| if (power_received_counter > 3) { |
There was a problem hiding this comment.
Added a ticket to change this to watchdog when Adhiraj makes a change, we will fix this
| power_received_counter++; | ||
| } | ||
|
|
||
| if (power_received_counter > 3) { |
There was a problem hiding this comment.
Magic number and duplicate logic. Maybe make into a prv function?
| s_regen_braking = false; | ||
| pca9555_gpio_set_state(&s_output_leds[HAZARD_LED], PCA9555_GPIO_STATE_LOW); | ||
| } else { | ||
| s_regen_braking = false; |
There was a problem hiding this comment.
Can just move s_regen_braking out of if
| if (get_power_info_power_state() != s_last_power_state) { | ||
| if (get_power_info_power_state() == EE_POWER_ON_STATE || | ||
| get_power_info_power_state() == EE_POWER_DRIVE_STATE) { | ||
| pca9555_gpio_set_state(&s_output_leds[POWER_LED], PCA9555_GPIO_STATE_HIGH); |
| } | ||
|
|
||
| // Update left/right LED | ||
| if (get_steering_info_input_lights() != s_last_lights_state) { |
There was a problem hiding this comment.
Perhaps this is my bad, but I'm pretty sure you don't need the () when you use the getters. They should just be #defines. Though if you like it's fine, but I'm pretty sure it serves no purpose
| if (get_drive_state() != DRIVE || get_pedal_output_brake_output()) { | ||
| new_cc_state = false; | ||
| } else { | ||
| if (cc_info & EE_STEERING_CC_TOGGLE_MASK) { |
| uint16_t batt_perc_val = get_battery_status_batt_perc(); | ||
| seg_display_set_int(&cc_display, s_target_velocity); | ||
| if (speed_kph >= 100) { | ||
| seg_display_set_int(&speed_display, (int)speed_kph); |
There was a problem hiding this comment.
Curious if this can overflow into a negative speed
There was a problem hiding this comment.
Hopefully we aren't doing > 255 km/h 😅, we might have bigger problems than speed overflow.
Kyle02
force-pushed
the
FWXV_250_Update_Drive_FSM
branch
from
70880a1 to
9a7eef7
Compare
Refactor Drive FSM to finalize the main functionality it needs to have