fix(color): Mixer list display incorrect after deleting all mixer lines.

[philmoz]

Fixes #4462

Restore code to handle empty mixer list (removed in PR #3999).

[philmoz]

Should be added to 2.9 if there will be another 2.9.x release.

[raphaelcoeffic]

@philmoz we really need a better convention. This code feels a bit silly.

[philmoz]

This code feels a bit silly.

Agreed; but like the delay handling code, it works and the lack of documentation makes changing it risky.

[raphaelcoeffic]

Agreed; but like the delay handling code, it works and the lack of documentation makes changing it risky.

Right, but we will still have to bite the bullet one day, I'm afraid. Otherwise that unfortunate situation will never end.

[raphaelcoeffic]

From what I can see, the storage code enforces the following conventions:

• a mixer line to valid as long as it's not all 0,
• valid mixer lines form a continuous block starting at index 0.

Wouldn't it be sufficient to maintain a simple counter with the number of valid mixer lines? The only potential pitfall I see is that you can potentially edit a mixer to be all 0 (at least it was the case last time I checked on the colour UI, not sure about the monochrome UI). We need to decide what should happen in such a case to enforce the absence of invalid mixer lines interleaved with valid ones.

Another consideration: the mixer itself uses some special treatment where srcRaw == 0. However, with a small twist, which IMHO should be removed:

      if (md->srcRaw == 0) {
#if defined(COLORLCD)
        continue;
#else
        break;
#endif
      }

This MUST be unified, we shouldn't have anything different in the mixer depending on colour vs monochrome UI.

Last but not least: I believe we have the same kind of issues with inputs.

[philmoz]

Wouldn't it be sufficient to maintain a simple counter with the number of valid mixer lines? The only potential pitfall I see is that you can potentially edit a mixer to be all 0 (at least it was the case last time I checked on the colour UI, not sure about the monochrome UI). We need to decide what should happen in such a case to enforce the absence of invalid mixer lines interleaved with valid ones.

There is a spare bit in the MixData struct that could be repurposed as an 'in-use/active' flag.

Another consideration: the mixer itself uses some special treatment where srcRaw == 0. However, with a small twist, which IMHO should be removed:

      if (md->srcRaw == 0) {
#if defined(COLORLCD)
        continue;
#else
        break;
#endif
      }

This MUST be unified, we shouldn't have anything different in the mixer depending on colour vs monochrome UI.

Would need investigation to see what the difference actually changes between color & B&W.

Last but not least: I believe we have the same kind of issues with inputs.

I would recommend handling all this as a separate PR (maybe more than one).

[raphaelcoeffic]

I would recommend handling all this as a separate PR (maybe more than one).

Agreed!

[philmoz]

      if (md->srcRaw == 0) {
#if defined(COLORLCD)
        continue;
#else
        break;
#endif
      }

On color radios you can set the md->srcRaw value to '---' (0) on any mix line.
On B&W radios (and in Companion) you can't do this.

The above code aborts the mixer loop on B&W radios because if srcRaw is 0 then it must be the end of the valid mix lines.
On color radios it just skips the the current mix line - there could be more valid mix lines to process.

Personally I don't understand why you would want a mix line with no source on color radios.

[raphaelcoeffic]

Personally I don't understand why you would want a mix line with no source on color radios.

Indeed, so the question becomes: should the line then be deleted? (as I believe it is on monochrome). Either way, it should be the same.

[elecpower]

Companion will allow as a result of radio conversion where a source may not exist on the new radio and thus the value is set to none. This way the user has a chance to select new valid sources or delete lines.
I have a vague recollection of uses pressing for this functionality especially for complex models and when many per settings file.

[philmoz]

Indeed, so the question becomes: should the line then be deleted? (as I believe it is on monochrome). Either way, it should be the same.

On B&W radios you can't change the source for a mix line to '---'.

Companion will allow as a result of radio conversion where a source may not exist on the new radio and thus the value is set to none. This way the user has a chance to select new valid sources or delete lines. I have a vague recollection of uses pressing for this functionality especially for complex models and when many per settings file.

That makes sense; but I don't see any need to be able to set the mix source to '---' on the radio itself.
This is the discrepancy - color radios can do this, B&W don't allow it.

Also in the current firmware code, if you load a converted model from companion onto a B&W radio where one of the mix lines has been set to '---' it will abort processing all mix lines as soon as this line is hit. Color radios just skip the affected line. We could change it to just skip these lines; but this might have a performance impact for B&W radios (it will always check all 64 mix lines).

[pfeerick]

That makes sense; but I don't see any need to be able to set the mix source to '---' on the radio itself.

How about the exact same situation as with Companion... you've copied a model onto a new radio, and the source doesn't exist? The only other reason I could think of is if you want to invalidate that line for some reason... effectively setting it to 0% / 1500us. Also I guess there could be instances were a conversion from an old storage format could result in a missing source (i.e. due to firmware options)?

This is the discrepancy - color radios can do this, B&W don't allow it.

That's the real issue there... they (color and bw) both need to be the same, once a decision is made as to the expected / desired behaviour. But let's have the UI not bugger itself up completely first?

[philmoz]

How about the exact same situation as with Companion... you've copied a model onto a new radio, and the source doesn't exist?

I meant when editing a mix line. Setting to '---' on model load is ok although I have not tested that this actually works on radios.