IRC Tramp CLI Power Tables

[SomeTestStuffDoesntWork]

Summary

• Added new configuration array to support IRC tramp configurations
  • Consists of five integers
  • Defaults to -1 on reset
  • Set by CLI via: tramp_pl_override <pl_idx> <pl_val_milliwatts> or tramp_pl_table <pl1_mw> <pl2_mw> ... <pl_n_mw>
    • tramp_pl_override can be used to override the default tramp tables without losing backwards compatability. For example, if a user defines PL 1 but no others, then only PL 1 is changed and the defaults for the other power levels are used. Alternatively, if the default table contains three power level entries but the user overrides up to entry 4, then the additional entry is added and the table in use can use up to PL 4. If <pl_val_milliwatts> is set to -1, the power level entry is ignored and the default for that entry is used. Again, this is a bit of a strange solution but it was done to allow for full PL configurability while retaining all prior tramp PL behavior when the new configurations are left unmodified.
    • tramp_pl_table can be used to define an entirely new table in one command to take the place of the defaults. If less than the max number of power levels are sent, then the remaining power levels in the table are filled with that of the last power level sent (IE tramp_pl_table 25 100 200 will fill PLs 3, 4, and 5 with 200 mw)
  • Can be used for all VTX powers up to what can be stored in a uin16_t
• Added tramp config dump command to view current tramp config/statistics
  • Can be run by issuing tramp_dump_config
  • Dumps:
    • Current number of valid entries in the power level table
    • Current power levels in use
    • Frequency as reported by VTX
    • Power as reported by VTX
    • Power requested by FC
    • Frequency requested by FC
    • Power index reported by FC
  • Allows users to determine if the VTX accepted the PL/frequency values expected and aids in diagnosing issues with stubborn transmitters

Testing

This has been tested with an ATOMRC F405 NAVI on the ground communicating with a stubborn VTX. This VTX was particulary impacted by the default tables as its PLs are set as 1, 2, and 3 in place of milliwatt values, making the defaults impossible to use. Changing PL entries 1, 2, and 3 to 1, 2, and 3 milliwatts respectively fixed this issue and the VTX correctly transitioned to higher PLs on command. Non-volatile storage was tested and saved values were recovered across power cycles. Configuration reset was also tested and all values initialized to -1 as expected. Additional board tests may be required.
Please Note: This contains a settings change and will most likely require a diff all + full erase + reapplication of original config in order to perform further testing.

Related

This may fix the following discussions/issues:

• Can't turn power level 5 on VTX #10208
• IRC Tramp RF Power Selection Enhancement #8340 (closed, but did not have a long-term solution. Contains the most comprehensive description of the issue)

Other Notes

I apologize in advance for any coding standards inconsistencies. I regularly use a slightly different set and some issues may have slipped through. Additionally, I have left a fair quantity of comments. Some of these may not be as useful, but I tend to err on the side of more comments rather than less to assist with later maintenance. I can remove the majority of these if they just add noise.

[github-actions]

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

• maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

• maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.

---

This is an automated suggestion to help route contributions to the appropriate branch.

[sensei-hacker]

Thanks for doing your first INAV PR! A lot of people will be very happy about this, because they've been asking for it for a long time. This is great, if it works out.

You mentioned comments. Let me comment on comments. :)
We have this code:

if(isEmpty(cmdline))
{
    cliPrint(parseErrString);
    return;
}

That's great code - totally self-explanatory. Someone who isn't even a programmer could likely read and pretty much understand that because it reads pretty much like English prose. If cmdLine is empty, print an error and return. Awesome.

THEREFORE, the comment repeating what the code says may be unnecessary noise that actually ends up making the code harder to read by making it longer:

if(isEmpty(cmdline))
{
    // Nothing sent, so fail and bail.
    cliPrint(parseErrString);
    return;
}

There's also the question there of -- are the comments before the code or after? Hmm, both?
Typically if a comment was needed, we'd put it before the code is is explaining:

    // Nothing sent, so fail and bail.
    if(isEmpty(cmdline))
    {
        cliPrint(parseErrString);
        return;
    }

But here there is nothing to explain - it's great, self-evident code.

Typically, comments should describe WHY the code is doing something that isn't obvious.
It would answer a question the reader might have - "why did they do that instead of just doing ...?"
This doesn't gain from a comment:

      sum = sum + newValue;

This certainly might!

        sum = sum + 0.0;

A comment could say 0.0 is added to make it a float. ( Though maybe explicitly casting to a float would be better in that example.)

We have a function called constructPowerTable(). Which contructs the power table, of course.
The comment "// Construct the power table" is just a distraction because the function is literally called constructPowerTable.

[SomeTestStuffDoesntWork]

Thanks for doing your first INAV PR! A lot of people will be very happy about this, because they've been asking for it for a long time. This is great, if it works out.

You mentioned comments. Let me comment on comments. :) We have this code:

if(isEmpty(cmdline))
{
    cliPrint(parseErrString);
    return;
}

That's great code - totally self-explanatory. Someone who isn't even a programmer could likely read and pretty much understand that because it reads pretty much like English prose. If cmdLine is empty, print an error and return. Awesome.

THEREFORE, the comment repeating what the code says may be unnecessary noise that actually ends up making the code harder to read by making it longer:

if(isEmpty(cmdline))
{
    // Nothing sent, so fail and bail.
    cliPrint(parseErrString);
    return;
}

There's also the question there of -- are the comments before the code or after? Hmm, both? Typically if a comment was needed, we'd put it before the code is is explaining:

    // Nothing sent, so fail and bail.
    if(isEmpty(cmdline))
    {
        cliPrint(parseErrString);
        return;
    }

But here there is nothing to explain - it's great, self-evident code.

Typically, comments should describe WHY the code is doing something that isn't obvious. It would answer a question the reader might have - "why did they do that instead of just doing ...?" This doesn't gain from a comment:

      sum = sum + newValue;

This certainly might!

        sum = sum + 0.0;

A comment could say 0.0 is added to make it a float. ( Though maybe explicitly casting to a float would be better in that example.)

We have a function called constructPowerTable(). Which contructs the power table, of course. The comment "// Construct the power table" is just a distraction because the function is literally called constructPowerTable.

This all sounds good to me. Took a pass through and cleaned up the comments.

Additionally, this commit should fix the unused static function errros from SITL builds. Had the commands in preprocessor conditionals but not the static functions used to service those commands. The functions should now follow the command definitions