Bq40z80 core1 #7
Conversation
talos/SmartBattery/src/main.c
Outdated
|
|
||
| uint8_t sbh_mcu_serial_num = *(uint8_t *) (0x101FF000); | ||
| core1_init(sbh_mcu_serial_num); | ||
|
|
||
| async_i2c_init(PERIPH_SDA_PIN, PERIPH_SCL_PIN, -1, -1, 400000, 20); | ||
| display_init(); | ||
| sht41_init(&sht41_sensor_error_cb, PERIPH_I2C); | ||
|
|
||
| sleep_ms(1000); |
talos/SmartBattery/src/main.c
Outdated
| panic("BQ40Z80 Init failed!"); | ||
| uint8_t retry = 0; | ||
| while (!core1_get_pack_mfg_info(&bq_mfg_info)) { | ||
| if (++retry > 5) |
There was a problem hiding this comment.
So this technically won't do anything since this doesn't block, so if we get to this point, it'll quickly fail 5 times rather than retrying 5 times (since it's up to core 1 for when it wants to retry).
I'd recommend some sort of logic to handle the battery being offline and recovering from that, but if that isn't possible, some sort of rate limit during initialization (but making sure we don't time out during safety ticking)
talos/SmartBattery/src/display.c
Outdated
| @@ -46,16 +93,104 @@ void display_show_stats(unsigned int serial, unsigned int soc, float voltage) { | |||
|
|
|||
| ssd1306_FillRectangle(0, 11, 79, 32, White); | |||
| ssd1306_SetCursor(2, 13); | |||
| snprintf(buf, sizeof(buf), "SOC %d%%", soc); | |||
| if (op_hl == 0) | |||
There was a problem hiding this comment.
Just for readability, this can be an enum rather than a raw int (or a define statement just to make the code easier to read)
talos/SmartBattery/src/display.c
Outdated
|
|
||
| char buf[16]; | ||
| ssd1306_SetDisplayOn(1); | ||
| ssd1306_Fill(Black); | ||
|
|
||
| ssd1306_SetCursor(0, 0); | ||
| snprintf(buf, sizeof(buf), "ID: 2023-%d", serial); // TODO: Replace with real number | ||
| snprintf(buf, sizeof(buf), "ID: 2023-%d", serial); |
There was a problem hiding this comment.
You never actually fixed the todo lol (this should be in MFG info though)
talos/SmartBattery/src/display.c
Outdated
| ssd1306_UpdateScreen(); | ||
| } | ||
|
|
||
| void display_show_stats(unsigned int serial, unsigned int soc, float voltage) { | ||
| static void display_show_option(unsigned int serial, uint8_t op_hl, bool dsg_mode) { |
There was a problem hiding this comment.
Any reason serial needs to be passed as a parameter rather than just pulling from core1 directly like you do for all these other values?
talos/SmartBattery/src/display.c
Outdated
| ssd1306_WriteString(buf, Font_11x18, Black); | ||
|
|
||
| ssd1306_SetCursor(86, 22); | ||
| snprintf(buf, sizeof(buf), "%.2fV", voltage); | ||
| if (dsg_mode) |
There was a problem hiding this comment.
Just a small comment, but do you think we could show something more useful here rather than charge or discharge mode, since we should know what we're doing to the battery, and I feel like if we have something else that we could show here, that would be better.
talos/SmartBattery/src/core1.c
Outdated
|
|
||
| static void core1_bq40z80_error_cb(const bq_error type, const int error_code) { | ||
| if (type == BQ_ERROR_SAFETY_STATUS) { | ||
| safety_raise_fault_with_arg(FAULT_BQ40_SAFETY_STATUS, error_code); |
There was a problem hiding this comment.
Do we ever show the safety status on the screen?
| struct __packed { | ||
| enum __packed cmd_type { | ||
| BQ_POWER_CYCLE, | ||
| BQ_CYCLE_COUNT, |
There was a problem hiding this comment.
We have some of these other commands, but I don't see them implemented
Verify bq40z80 in core 1 and display code