Presets: ignore CLI error messages caused by resetting defaults #3635
Conversation
This comment has been minimized.
This comment has been minimized.
|
SonarCloud Quality Gate failed.
|
|
Do you want to test this code? Here you have an automated build: |
|
AUTOMERGE: (FAIL)
|
haslinghuis
requested review from
McGiverGim,
chmelevskij,
asizon and
nerdCopter
|
I can reproduce pretty much on any FC.
Same errors will pop when you load backup on presets tab. They appear when 4.4 build doesn't include, for example, baro, or camera control, but the config file for the board has it. For example foxeer F7 v1-V4 |
| @@ -72,7 +73,7 @@ export default class CliEngine | |||
| } | |||
|
|
|||
| close(callback) { | |||
| this.send(this.getCliCommand('exit\r', ""), function () { //this.cliBuffer | |||
| this.send(this.getCliCommand('exit\r', ""), function () { //this._cliBuffer | |||
There was a problem hiding this comment.
Seems unused.
| this.send(this.getCliCommand('exit\r', ""), function () { //this._cliBuffer | |
| this.send(this.getCliCommand('exit\r', ""), function () { |
| if (text.startsWith("###ERROR")) { | ||
| this.writeToOutput(`<span class="error_message">${text}</span><br>`); | ||
| this._cliErrorsCount++; | ||
| if (this._lastCommandIsDefaults) { | ||
| this.writeToOutput(`<span class="warning_message">${text}</span><br>`); | ||
| } else { | ||
| this.writeToOutput(`<span class="error_message">${text}</span><br>`); | ||
| this._cliErrorsCount++; | ||
| } | ||
| } else { | ||
| this._lastCommandIsDefaults = text.toLowerCase().includes(CliEngine.s_outputResetting); |
There was a problem hiding this comment.
Is plain text used for logging and the actual logic here?
There was a problem hiding this comment.
I didn't get the question
There was a problem hiding this comment.
So instead of sending something like this
{
type: 'error',
message: '###ERROR some error message',
}
we have plain string which is used for display and hope that this value is formatted correctly when we do the logic. Like the meaning of the message is not separate from display.
Like, if for example this would be localised at some point, wouldn't ###ERROR become ###BŁAD in polish
There was a problem hiding this comment.
I see yeah, it could be improved. This part is straight copy-paste from CLI tab.
haslinghuis
dismissed
their stale review
I've also now found some errors in presets for 4.5 - so I don't think it's a good idea to mask them.
|
@haslinghuis it's not masking all errors, just the ones coming right after "defaults nosave". If presets got errors, they still be shown. |
|
@limonspb we should rather fix the underlying problem in firmware and look at each error (as we still can see the errors in cli). |
I wish it was possible for 4.4 But 4.4 had this problem since the beginning. I think it's because default config might have some commands like camera control, that might not be included in the actual firmware build. |
this PR is mainly for BF 4.4 where resetting to defaults causes CLI errors.
This problem makes the CLI dialog to show up every time the user hit "save" on the presets tab, or loads a backup.
The solution is to ignore errors that's popping right after resetting to defaults.
While working, i also noticed a bug that presets CLI messages timeout are being ignored.