Skip to content

Fix footpad unknown switch state - #98

Open
0x41kravchenko wants to merge 1 commit into
thankthemaker:masterfrom
0x41kravchenko:fix-footpad-unknown-switch-state
Open

Fix footpad unknown switch state#98
0x41kravchenko wants to merge 1 commit into
thankthemaker:masterfrom
0x41kravchenko:fix-footpad-unknown-switch-state

Conversation

@0x41kravchenko

Copy link
Copy Markdown

This PR should fix unknown switch state error that causes status bar behaviour described in this issue: #88

switchState set in Canbus.cpp was not corresponding to switchState mapper in LightBarController.cpp

rESCue/src/CanBus.cpp

Lines 632 to 643 in ccd3f1f

switch(switchState)
{
case 0:
vescData->switchState=0;
break;
case 1:
vescData->switchState = (vescData->adc1 > vescData->adc2) ? 1 : 2;
break;
case 2:
vescData->switchState=3;
break;
}

switch (intState) {
case 0:
return AdcState::ADC_NONE;
case 1:
return isAdc1Enabled ? AdcState::ADC_HALF_ADC1 : AdcState::ADC_HALF_ADC2;
case 2:
return AdcState::ADC_FULL;
default:
ESP_LOGE(LOG_TAG_LIGHTBAR, "Unknown switch state");
}

@thankthemaker

Copy link
Copy Markdown
Owner

Thanks for your PR, I really appreciate it.
Do you have an idea from which firmware version onwards the behavior has changed? Maybe it makes sense to enhance the PR to support both newer and older versions

@0x41kravchenko

Copy link
Copy Markdown
Author

@thankthemaker I'm glad to contribute! Thank you for the project!
AFAIK this wasn't changed in the float/refloat packages for ages it's just how rESCue/src/CanBus.cpp maps the switchState. When we receive switchState value 2 on can bus it is mapped to vescData->switchState=3 and since current rESCue/src/LightBarController.cpp implementation doesn't have case 3 in switch (intState) it logs Unknown switch state.
Float/refloat packages are using value 2 (for compatibility) when both sensors are activated:
vesc_pkg-float/blob/main/float/float/footpad_sensor.c
refloat/blob/main/src/footpad_sensor.c

After looking in float/refloat code from above now I'm actually thinking that the fix should be applied in rESCue/src/CanBus.cpp
I mean to remove this switch statement completely in order to make the logic align with VESC UART and with what rESCue/src/LightBarController.cpp expects. I'm not sure why this switchState mapper in rESCue/src/CanBus.cpp was added in the first place though if no other place in the code expects value 3.
Please let me know what you think.
The original fix in PR fixed the issue for me since I was using CAN and actually never tried UART.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants