edgetx
edgetx copied to clipboard
EdgeTX
Interactive/progressive checklist
Summary of changes: Adds an option to display check-boxes at the beginning of each checklist / model notes line.
- Item to be currently processed has selected (active) check-box, confirmed item - selected checkbox, unconfirmed item - empty.
- You can only advance by one in the checklist (by clicking ENTER - rotary encoder click).
- You can normally scroll the checklist if it exceeds screen size and in addition it is automatically scrolled when currently processed item is at the bottom (or row above bottom) and there are more items available.
- You can remove displaying and processing of some items by placing
=sign as a first character in a line - You can mark only items, that are displayed on a screen (no off-screen item confirmation)
- Interactive checklist loaded on model selection (and after reset flight) might be closed only when complete (additional ENTER click after last item completed), see #133
- Interactive checklist loaded from View Notes is closed by pressing EXIT/RTN, no check-boxes are displayed in the view.
Notes:
- All changes were tested only on following hardware: Radiomaster TX12
- Changes for colorlcd tested only in simulator
- Tested that opening yaml file without positions for the settings is safe (defaults to this option disabled - backwards compatible)
Nice... so this goes towards resolving https://github.com/EdgeTX/edgetx/issues/133 for B&W, and gives a model for how it could be done for colorlcd as well.
So colorlcd does not have such an option (I am getting mine in a few weeks - delivery from CN)? Actually as of https://github.com/EdgeTX/edgetx/pull/1417/commits/8755465f532999d966c893db0285672fd82b214d the checklist can be closed by clicking EXIT/RTN button (at any state). I was considering to make this not even seeing that issue. Work in progress - should be one commit hopefully soon ;)
No, colorlcd is basically the same as B&W in that respect - it can show the model notes file as pre-flight checklist, but there is no interactivity.
So for now it should resolve #133 for B&W radios - I will probably go with color version as I get my radio, cause otherwise I have no way to test it. A thing to discuss is:
- Interactive checklist loaded from View Notes menu might also be closed by pressing EXIT/RTN
It might be easily changed with one condition, and I think it is not necessary to have this option to enable/disable in the menu. If someone might comment if my approach on this is right or not - I would appreciate.
Just to clarify - so when you have it set as interactive, it would be interactive on powerup/model switch, but when you view notes, not-interactive? If so, I think this is the correct behaviour - as you said you wanted to view the notes, not redo the checklist. You can reset the flight if you want to do that, as that will go through all the pre-flight checks again.
When you view notes - check-boxes are still displayed and you can still go-through them and complete the checklist but in addition you can close checklist by pressing RTN/EXIT (so in addition not instead).
I'm not really fussed either way. I would have probably gone with not showing the checkboxes in the view notes dialog... but lets see if others have any thoughts on that.
That would be reasonable, as it would clearly indicate this this window is to be closed in the other way. Also a very simple change :smile: Of course if the way I manage this is OK.
Maybe you should rename checklistInteractiveBW to checklistInteractive because it is also introduced for the colorLCDs
@wimalopaan I will rename this setting, as (as you can see beginning of the discussion) I have thought that colorlcd already have this feature hah :smile: And what about loading checklist as "View Notes" ?
I think you should try to do this on the color lcd radios also. This just UI stuff that can be perfectly developed with simu/simulator, probably even better than on the radio itself. Tests on the real hardware can be done by he people who have access to all the different kinds of radios. I made a feature only of B/W radios without ever having one in my hands. Even when you get your TX16S there is also NV14 that has to be tested, which has a completely different screen layout and in the future PL18, which has the same display as the NV14, but in landscape mode.
@gagarinlg I'd rather leave it for a separate PR - the issue is not the layout itself, but the way radio handles some action (like menu enter/exit). This is only different between colorlcd and B&W.
And what about loading checklist as "View Notes" ?
In my personal view I would prefer to see the "View Notes" with the previosly left state, because this might be important. But I do not see a need to complete the checklist at this time.
Updated as we discussed:
- config value name changed
- added padding field
- when loaded as View Notes no check-boxes and closing using EXIT/RTN button
- included screen in the commit comment
Companion support added and seems like a complete feature for both B&W and colorlcd. This should resolve #133 if progressive checklist is acceptable.
Do you have a preferred order for the review/merging of #1395, #1417 (this PR) and #1425? I'm thinking this one first since it's clear and is the active one, and then either of the other two, and then the remainder. There is just a problem with merge conflict for each due to the yaml config structures needing to be updated each time...
I think we may go with this one first, as it is a complete one and yesterday I resolved all conflicts with main branch. These two additional PRs are not as complete as this one, still need to add companion support. And also they are quite compact, so would be easier to go with resolving conflicts for them later.
I just tested this PR.
At this stage every single line represents a checkable item. I think this could bei improved. In #133 I was thinking of a checklist with checkable and non-checkable items.
Maybe checkable items could start with [ ]:
[ ] Check
No Check
and after check should be rendered as:
[x] Check
No Check
@wimalopaan I was thinking about it but for such a feature we would need to agree how to mark checkable/non-checkable items.
From programming point of view the best solution would be to use a single character at te beginning of the line. I'd personally go for + for checkable items and space/- for non-checkable ones.
With such approach I can check first char of the line and decide to display (or not) place for "tick" as well as "auto-check" such lines (as I do not want to allocate memory to remember which lines were completed - I'd simply automatically mark non-checkable ones as checked while not displaying any check.
What do you think about it (bot from UX and back-end point of view)?
Or other approach - by default all items are chackable unless first character in line is a space/- - this would allow to easy use existing checklist with all items checkable.
Do you have a preferred order for the review/merging of #1395, #1417 (this PR) and #1425? I'm thinking this one first since it's clear and is the active one, and then either of the other two, and then the remainder. There is just a problem with merge conflict for each due to the yaml config structures needing to be updated each time...
Considering our last discussion in this PR, and recent commits to others mentioned - I think now it makes more sense to consider these two others PR to merge first, then this one.
Another question - as this feature gets more complex (it involves special file preparation) - where should I make a complete description (aka. User Manual) of the solution?
Also - considering not all items are chackable - I see a need to use proper checkboxes in colorlcd implementation.
I don't see much difference in using
+ Chekable
[ ] Checkable
Uncheckable
concering the parsing, but as a matter of style I would prefer the [ ].
When rendering the text on colorLCD I fully agree to replace the textual [ ] or [x] with its graphical counterpart.
@wimalopaan Would be grateful if you give your opinion about the feature now.
Finally I decided to use a single-character (as it greatly simplifies the code responsible for omitting non-checkable items).
So any line beginning with = character would be non-checkable (and would be skipped), while any other line would be checkable. The character indicating non-checkable lines might be easily changed in the source code if needed. I don't see reason to define this character in configuration of the radio.
It takes into consideration also the fact, that user might want to enable interactive checklist for previously prepared .txt file, so I decided to have checkable items by default.
Sorry for the delay, didn't see the notification for this. I'll trade you review of this for testing of #1548 since I know you have a TX12 also. 😁
Sorry for the delay, didn't see the notification for this. I'll trade you review of this for testing of #1548 since I know you have a TX12 also. 😁
@pfeerick Testing done, If you had something to test in future - feel free to tag me - not always having time to go through issues/PRs. And also starting a day before yesterday - I am an owner of Eachine TX16s too ;)
btw, I was only holding reviewing this PR as https://github.com/EdgeTX/edgetx/pull/1481 needs to go in for 2.6.1 and whichever PR goes in first will need the other rebased... but I think it needs more work for companion anyway so this can go in first. So I will review this one today, and possibly your other one as well and then can do a single update to the other PR.
Right, so finally playing with this on TX16S. Companion side of things seems fine. I've noticed a few... quirks... in behaviour ... probably because I'm using this the wrong way. ;)
I know you have it so that adding = as the first character inhibits the checkbox... but to my way of thinking it would be better to have it requiring a = as the first character. What was the benefit of doing it this way?
Blank links also need some catering for also, for colorlcd at least, as they seem to behave as if there is a checkbox still (consuming a ENTER) - it made me think at first that scrolling when checking boxes seems would break, but that was because blank lines would give no idea as to where the next 'check' was.
There is no way to get out of the checklist? This was a showstopper in conjunction with the prior - as it was nearly impossible to get out of the checklist. RTN/Long RTN to exit? I can use touch exit, but that won't non-touch radios.
Scrollbar in the model notes view is broken - although admittedly I think it acts a bit odd with 2.6.
i.e. this is the dummy checklist I'm using atm... basically a mock model notes that has had the checklist added... as you can see it would be much... neater... with use = for the four check items. That now makes me wonder as to whether a separate file would be better for checklist... something to think about in the future?
=FPV 250
=3S/4S Battery
Check Props
Check Battery
Check GPS
Check Failsafe
=Switches:
=SF\up = Arm
=SD\up = Self-Level
=SD- = Horizon
=SD\dn = Acro
=SC\up = Level
=SC- = Horizon
=SC\dn = Acro
=SB\up = None
=SB- = Buzzer
=SB\dn = GPS Rescue