EndlessCadence
100 W
- Joined
- Aug 22, 2018
- Messages
- 217
Thanks, then I'll complete the PR to fix the potential safety issue with throttle/temperature sensor.casainho said:The code seems ok to me, and I like it lie being more modular.EndlessCadence said:Btw, do you have time to review this PR? https://github.com/OpenSource-EBike-firmware/TSDZ2-Smart-EBike/pull/16
I've created a preprocessor switch/macro to choose between throttle and non-throttle version of the firmware. Release script builds both and the hexfiles (0.14.1) are included as well.
I suggest you to keep the right variable names. You pass the address of ui8_adc_battery_target_current but inside you call it ui8_target_current. On the firmware, there are moments were current is used in amps and others where current is ADC steps. The right name should help to read correctly the code and avoid mistakes.Code:apply_throttle (ui8_throttle, &ui8_startup_enable, &ui8_adc_battery_target_current); static void apply_throttle (uint8_t ui8_throttle_value, uint8_t *ui8_motor_enable, uint8_t *ui8_target_current) #if THROTTLE {
Regarding the variable names, I did this on purpose to clearly indicate it's a local variable/parameter. In my opinion the code has way too many global symbols and variables but I understand your concern and will take care of it :wink: