Fix SPI FC problem with looping connection loss, reestablishment #3079
Conversation
ba6dee9 to
fc44c99
Compare
fc44c99 to
d102753
Compare
CapnBry
left a comment
There was a problem hiding this comment.
Great, thanks for getting this off my todo list!
Just to make sure we're all on the same page though, @JyeSmith added forcing sendEepromWrite to true in VtxTriggerSend() saying:
- Fix eeprom writes by making sure they are sent on the first VTx MSP sends after Tx module boot.
But sendEepromWrite is true when a TX module boots, and VtxTriggerSend() is not called at all when a Tx module boots so it should always force write after a TX boots? This line was intentionally not included here when I wrote this code, it was not an omission. I feel like the line might have been added too hastily as possibly part of the VtxTriggerSend on connect, which should not have been added.
It has been a long time, and I spent 30 minutes pondering this, but I believe my reasoning was that forcing an EEPROM write on a flight controller was considered dangerous at the time so forcing a write is reserved strictly for when starting from UNKOWN (as close as we can determine that this is a new connection, although the worst part of VTX Admin is that it also does it at low LQ).
I'm ok with leaving it now, as after this PR it should only force it true by user interaction, but I want to make sure this is the intended behavior we want, since it was not the intended behavior we wanted in the early days of VTX Admin due to the danger of EEPROM write freezes on the FC.
|
Thanks for updating with the new comments and names! I gave it the full testing of everything I could think of today with debug logging
I have all these changes made if you'd like me to push or here is the full diff diff --git a/src/lib/LUA/tx_devLUA.cpp b/src/lib/LUA/tx_devLUA.cpp
index 55738cc7..1fb9c694 100644
--- a/src/lib/LUA/tx_devLUA.cpp
+++ b/src/lib/LUA/tx_devLUA.cpp
@@ -907,7 +907,16 @@ static int event()
setLuaTextSelectionValue(&luaVtxBand, config.GetVtxBand());
setLuaUint8Value(&luaVtxChannel, config.GetVtxChannel() + 1);
setLuaTextSelectionValue(&luaVtxPwr, config.GetVtxPower());
- setLuaTextSelectionValue(&luaVtxPit, config.GetVtxPitmode());
+ if (config.GetVtxPower() != 0)
+ {
+ // Pit mode can only be sent as part of the power byte
+ LUA_FIELD_SHOW(luaVtxPit);
+ setLuaTextSelectionValue(&luaVtxPit, config.GetVtxPitmode());
+ }
+ else
+ {
+ LUA_FIELD_HIDE(luaVtxPit);
+ }
if (OPT_USE_TX_BACKPACK)
{
#if defined(GPIO_PIN_BACKPACK_EN)
diff --git a/src/lib/VTX/devVTX.cpp b/src/lib/VTX/devVTX.cpp
index 5990e8f8..add2e649 100644
--- a/src/lib/VTX/devVTX.cpp
+++ b/src/lib/VTX/devVTX.cpp
@@ -61,8 +61,9 @@ void VtxPitmodeSwitchUpdate()
else if (pitmodeAuxState != newPitmodeAuxState)
{
pitmodeAuxState = newPitmodeAuxState;
- sendEepromWrite = false;
VtxTriggerSend();
+ // No forced EEPROM saving of Pit Mode
+ sendEepromWrite = false;
}
}
@@ -123,7 +124,6 @@ static int event()
if (connectionState == disconnected && VtxSendState != VTXSS_DEBOUNCING && VtxSendState != VTXSS_UNKNOWN)
{
- CRSF::ResetMspQueue();
VtxSendState = VTXSS_DEBOUNCING;
return VTX_DISCONNECT_DEBOUNCE_MS;
}
@@ -135,7 +135,17 @@ static int timeout()
{
if (VtxSendState == VTXSS_DEBOUNCING)
{
- VtxSendState = connectionState == disconnected ? VTXSS_UNKNOWN : VTXSS_CONFIRMED;
+ if (connectionState == disconnected)
+ {
+ // Still disconnected after the debounce, assume this is a new connection on next connect
+ VtxSendState = VTXSS_UNKNOWN;
+ sendEepromWrite = true;
+ }
+ else
+ {
+ // Reconnect within the debounce, move back to CONFIRMED
+ VtxSendState = VTXSS_CONFIRMED;
+ }
}
if (VtxSendState == VTXSS_UNKNOWN || VtxSendState == VTXSS_MODIFIED || VtxSendState == VTXSS_CONFIRMED)
@@ -163,6 +173,7 @@ static int timeout()
}
else
{
+ CRSF::ResetMspQueue();
VtxSendState = VTXSS_UNKNOWN;
} |
|
@CapnBry I'm happy for you to push your updates |
…nd, remove pidmoode edge case
CapnBry
left a comment
There was a problem hiding this comment.
You're the best, PK! Thanks so much for taking this on and getting it done during the holidays. Appreciate your work.
|
@JyeSmith I won't merge till you let us know. Also if you prefer the 10s debounce I'm happy to changer that back too. |
This is a fix for the issue with Betaflight ELRS SPI flight controllers that exhibit an issue when using the VTX admin.
Upon RF connection, the TX sends the current VTX settings and a EEPROM save, which causes the link to drop. The connection is reestablished, and we send another VTX packet with EEPROM save ad infinitum.
The fix is to properly detect disconnected state and start the debounce timer so if the connection is reestablished before the debounce time (1 second) the VTX Admin packets are not sent again, thus stopping another disconnect/reconnect cycle.
This issue was introduced in PR #2965 and fixes #2976