edgetx
edgetx copied to clipboard
feat(firmware) - Support 'Source' invert option.
As a pre-requisite for #4452 add the ability to invert a 'Source'.
Currently supports Inputs, Mixes, Logical Switches and Special/Global functions on color radios.
TODO:
- [x] B&W UI
- [x] Companion support
- [ ] Add other 'Sources'
@philmoz suggest not starting Companion work until #4406 is merged as it has lots of changes in this area and likely more depending how well testing goes
@elecpower When you have time I would appreciate your thoughts on the way I have done the Companion support.
Will do
@philmoz First thoughts.
- overall looks okay.
- have you checked binary conversions to ensure they are not offset by the inverted entries?
- am I correct in assuming: -- the new inverted data model is POC as do not need both? -- the RawSource inverted property is POC as method used for +/- RawSwitches would be appropriate?
- the double length source list in drop-downs is not the best especially when there is currently no none ie '---' in the list to be set as the starting position as done for switch list.
- Most users will want to use positive values and making them scroll past inverted entries is likely to generate pushback.
- I know there is not meant to be a none source but it happens in radio to radio conversions and if someone manually edits a yaml file so why not include in list?
Will the choice of '!' to signify inverted be confusing as we currently use it to signify 'not' for switches?
Thanks for the feedback.
- have you checked binary conversions to ensure they are not offset by the inverted entries?
I don't have any files to test this with; but I don't believe there would be any issue. The only place I know of where a negative 'source' value is when a GV is used for weight/offset etc. Changing this is not currently part of this PR.
-- the new inverted data model is POC as do not need both?
Yes, not all 'source' fields are changed yet and there may be reasons why some are not (still thinking through this).
-- the RawSource inverted property is POC as method used for +/- RawSwitches would be appropriate?
Using a negative value for RawSource index won't work (I tried this initially). The index value for a each source type always starts at 0 and since -0 == 0 you can't indicate inversion with just a negative value.
- the double length source list in drop-downs is not the best especially when there is currently no none ie '---' in the list to be set as the starting position as done for switch list.
- Most users will want to use positive values and making them scroll past inverted entries is likely to generate pushback.
I agree it's not user friendly. Still thinking of better ways to handle this.
- I know there is not meant to be a none source but it happens in radio to radio conversions and if someone manually edits a yaml file so why not include in list?
Allowing a '---' entry in conversion is needed so people can fix it. Allowing it to be entered as a legitimate value is asking for trouble (and support questions).
Will the choice of '!' to signify inverted be confusing as we currently use it to signify 'not' for switches?
It's also used when selecting a GV for weight/offset etc so should not be too confusing.
I don't have any files to test this with; but I don't believe there would be any issue. The only place I know of where a negative 'source' value is when a GV is used for weight/offset etc. Changing this is not currently part of this PR.
My concern is that when reading the binary file offsets are used to map to RawSource. I have some old bin files so can check that when this PR firms up some more. Should not be too hard to fix as there are enough fudges now in the bin parser.
Using a negative value for RawSource index won't work (I tried this initially). The index value for a each source type always starts at 0 and since -0 == 0 you can't indicate inversion with just a negative value.
Hum. Let me think about this some more.
It's also used when selecting a GV for weight/offset etc so should not be too confusing.
Got me on this one. Should not code check late at night ;-)
Any thoughts on this as an alternative for the source UI?
Any thoughts on this as an alternative for the source UI?
Me no like. It would also mean adding the inverted checkbox to every source usage case in the gui for consistency. How about setting the initial index to list size / 2 which should select 1st non-inverted if my maths is right.
Any thoughts on this as an alternative for the source UI?
Me no like. It would also mean adding the inverted checkbox to every source usage case in the gui for consistency. How about setting the initial index to list size / 2 which should select 1st non-inverted if my maths is right.
There aren't that many of them so it's not a huge effort. It also means we don't need two models for the popup list.
There aren't that many of them so it's not a huge effort. It also means we don't need two models for the popup list.
I haven't thought this through yet so apologies may be necessary, but why do we need two models in the final solution? We can have one model and a context filter to filter out inverted entries if there are such use cases.
I haven't thought this through yet so apologies may be necessary, but why do we need two models in the final solution? We can have one model and a context filter to filter out inverted entries if there are such use cases.
BFI. I don't know QT well enough to do that.
BFI. I don't know QT well enough to do that.
I can do that if there is a need.
Also having a separate check box means we couldn't use autocombobox widget and that would mean more changes.
How about setting the initial index to list size / 2 which should select 1st non-inverted if my maths is right.
That means the combo box gets populated with the first entry when creating a new item. E.G. currently if you create a new Input the Source box is empty - if the default index is set then it gets populated with the first source item in the list.
Using a negative value for RawSource index won't work (I tried this initially). The index value for a each source type always starts at 0 and since -0 == 0 you can't indicate inversion with just a negative value.
Most switches are one based to cater for negative/not. Why not all is a ETX legacy I assume based on the logic it would be illogical to negate them. However it then requires shifting the index which is a trap for the newbies and when one gets forgetful.
I'm leaning towards biting the bullet and changing RawSource to one based except for NONE and implement inverted filter context whilst at it. Better future proofing solution IMO.
Whilst doing that I'd create a new source type for timers as it is on my todo list to remove from special type.
Also change all RawSwitch types to one based.
I suspect the biggest effort will be the binary fudging required and testing.
Maybe use '-'? Checkboxes are not always BW UI friendly
Closed - merged into #4722