Dashboard 25 #147
Dashboard 25 #147
Conversation
There was a problem hiding this comment.
I have not looked at main or LCD files yet but have some initial thoughts on the nextion HMI screens and driver profiles:
- For the driver profiles, instead of saving those to flash (or in addition to doing that), I think we should have an option to save the cooling settings currently used to flash. Additionally, we should have a button to retrieve the previously stored cooling configuration. This will be very nice to have on track. This could even be as simple as hitting the "ok" button on the race page, but something that we can use to very quickly restore a saved or default cooling configuration would be much appreciated.
For the Nextion HMI:
I notice you made everything have a black background, and I don't know if I like that idea - here is why:
I think it's fine to have a couple things have a black background. For example, the race page is nice to keep black for the high contrast. However, I don't know if I'm a fan of having the fault pages be black as well. Those pages are meant to stand out because something unexpected happened to the car. This to me means that a glaring white, yellow, or red page is far easier to tell apart from a bunch of black pages, and will instantly tell me the severity of the fault.
I don't think the APPS page is going to do too much good, the status information is already available through faults, and we mainly need the BSPD potentiometer values. Even if the PCB is not ready to support this, if Justin can get a rev in time then we should be able to easily calibrate BSPD.
Additionally, idk if you have found this annoying, but when you have text already in an HMI file it appears as soon as you change pages and exists there for a short period of time, which kind of annoys me.
After just glancing at you LCD changes though, it looks a lot better than before, so good work on that, I'll take a closer look at it tomorrow.
source/dashboard/pedals/pedals.c
Outdated
| static const uint32_t* PROFILE_FLASH_START = (uint32_t*)ADDR_FLASH_SECTOR_3; | ||
| static volatile uint32_t* profile_current_address; | ||
|
|
||
| int writeProfiles() { // TODO switch to EEPROM | ||
| profile_current_address = (volatile uint32_t*)PROFILE_FLASH_START; | ||
|
|
||
| if (FLASH_OK != PHAL_flashErasePage(PROFILES_START_SECTOR)) { // !! Fix the crash here | ||
| return PROFILE_WRITE_FAIL; | ||
| } | ||
|
|
||
| for (uint8_t i = 0; i < NUM_PROFILES; ++i) { | ||
| if (FLASH_OK != PHAL_flashWriteU32((uint32_t)profile_current_address, | ||
| *(uint32_t*)&driver_profiles[i])) { | ||
| return PROFILE_WRITE_FAIL; | ||
| } | ||
| profile_current_address++; | ||
| } | ||
|
|
||
| return PROFILE_WRITE_SUCCESS; | ||
| } | ||
|
|
||
| void readProfiles() { | ||
| uint32_t read_address = ADDR_FLASH_SECTOR_11; | ||
|
|
||
| for (uint8_t i = 0; i < NUM_PROFILES; ++i) { | ||
| uint32_t *data = (uint32_t *)&driver_profiles[i]; | ||
| *data = *((uint32_t *)read_address); | ||
|
|
||
| read_address += sizeof(driver_profile_t); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this shouldn't be in pedals.c. Move it to main imo
source/dashboard/pedals/pedals.c
Outdated
| driver_profile_t driver_profiles[4] = { | ||
| {0, 10,10,0}, | ||
| {1, 10,10,0}, | ||
| {2, 10,10,0}, | ||
| {3, 10,10,0} | ||
| }; |
There was a problem hiding this comment.
What is your plan with this? I'm not sure how effective this will be, but if the drivers thought it was good then I'm sure its fine.
There was a problem hiding this comment.
Some drivers are taller -> their foot rests on the pedal naturally -> adjustable pedal thresholds.
Can also be used to store other settings besides threshold, although its mainly proof of concept for now.
There was a problem hiding this comment.
What I mean to say is what do these numbers mean? From first glance, it would indicate that by changing the pedal from 10 to 9 for example, you sacrifice 10 percent of pedal travel. I would rather have the full range of throttle in this number for finer control of exactly how much pedal we sacrifice, because the driver may only be causing 1-2% throttle by resting his foot on it.
There was a problem hiding this comment.
They are thresholds (percentage from 1-100) but since they are stored as uint8 can also be x/255 for finer tuning. So currently they are all just defaulted to 10% pedal travel.
- Also those values are not yet integrated with apps, and will not be until the EEPROM stuff is figured out
Will write better comments though for sure.
There was a problem hiding this comment.
I think just naming the variables better in the struct definition will suffice. Maybe something like throttle_deadband_percentage, or something that tells me that:
- It is elongating the 0% threshold of the pedals
- Units
Also don't do anything with the brakes. If the driver can't hit full brakes I think that's important to know, and they should not be dragging the brakes anyway.
Change Summary
TODO: