edgetx
edgetx copied to clipboard
EdgeTX
Model Labels Selector
This PR brought to you by @kevinkoenig, @raphaelcoeffic and @dlktdr.
Models now have a label associated with them instead of a Category. Each model can belong multiple labels.
Models are now stored in /MODELS/labels.yml
- All models are scanned at startup. The YAML files are opened and the important information is read.
- After the first scan a model won't be opened on boot to keep startup fast.
- labels.yml keeps track of which files need to be re-scanned. Also keeps all labels added and their order. The file can't be hand-edited it's refreshed on every boot and when models or labels have changed
- Keeps extra RF data of each model & checks if the receiver id is used
GUI
- Select labels on the left bar
- Models on right
- Sort order at bottom of the list
- Long press on labels or models for extra options
To Do
- [x] Unplugging USB needs to cause a re-scan - I guess it actually was working!
- [x] Progress bar on rename a label as it can be slow if there are lots of labels
- [x] NV14 Portrait mode
- [x] Auto create a Favorites label if no labels found
- [x] Companion modifications - EDIT Will be in a separate PR
- [x] Fix Hardware keys in Listbox & Page buttons
- [x] Work PR#1467 into new selector
- [x] Refresh receiver ID on change in model_setup
- [x] Replace multiple labels selection with AND. Want e.g Planes AND Favorites to show not OR
- [x] Delete label causes a crash
- [x] Multiselect menu for label selection, used model_setup and model_select
- [x] Progress bar on rename/delete labels
- [x] Label name dialog buttons in wrong place
@kevinkoenig and @dlktdr can you please add to your To Do list Companion changes
I've not tried this PR yet, so my apologies if this has already been dealt with. Can you also consider how to make it so it is harder to accidentally switch models? i.e. the current model in 2.6 is tap model, menu shows, tap select model. #1467 is a really good step forward, but I think (and it was mentioned in another issue but I can't find it again now) it would be better to make it so double tap is the switch to model trigger. Thus also making the "press/tap and hold shows the menu" a consistent behavior also.
I've not tried this PR yet, so my apologies if this has already been dealt with. Can you also consider how to make it so it is harder to accidentally switch models? i.e. the current model in 2.6 is tap model, menu shows, tap select model. #1467 is a really good step forward, but I think (and it was mentioned in another issue but I can't find it again now) it would be better to make it so double tap is the switch to model trigger. Thus also making the "press/tap and hold shows the menu" a consistent behavior also.
I have made some modifications to help with this. After some changes a short press on a model does nothing. A long press brings up the selection menu. It used to bring up the labels selector but is now in a sub menu. I feel like you really have to try now to change the model... although I still don't know why you would even try if your model is connected and flying :)
You can now also select multiple labels at a time. Still needs work on the hardware keys.
I will incorporate PR#1467 as I think this is a good idea too,
Me also... but it happens, so I'm told.
That is perfect! :) Having the Create / New Model broken out of the menu into it's own button also alleviates one of the the other problems... the "accidentally created a new model" one - plus it really didn't make sense when you think of it - since creating a new model is not relevant in the context menu of another model. Looking forward to trying this out in a couple of days.
So here is what I currently have for the Labels in Companion. Been quite a few code changes and some learning so have not added to the pull yet.
I Still need to incorporate writing the labels.yml file, move, add, delete, rename. but this is the basic plan..
Feedback before I get too carried away?
As far as I'm concerned, that looks perfect - simple and effective. Do you have it smart enough (yet?) to show when labels are relevant (i.e. colorlcd), but not when it's not (i.e. B&W)?
Companion looks good to me. Remember to add the handling of the labels.yml file in the functions Read and Write Models Settings; reading and writing etx file format; and board conversions.
As far as I'm concerned, that looks perfect - simple and effective. Do you have it smart enough (yet?) to show when labels are relevant (i.e. colorlcd), but not when it's not (i.e. B&W)?
Yes that part does work.
@elecpower - Can read labels.yml. writing not done and conversions need to be verified. I'll post it soon and let you look at the code for some suggestions.. after I fix up a glaring botch. ;)
For conversion testing. Does anyone have a few backups .otx and .etx from various versions and radios I can test with? I'm limited on my backups I have from my 2019x9d+ and tx16s in OTX days. Would be helpful to verify for radio side too.
Hey @dlktdr @kevinkoenig ... health and wellbeing check :) How are things progressing with this PR? I was hoping this would make 2.7 but it's looking like it might miss that, and be early 2.8?
Hey @dlktdr @kevinkoenig ... health and wellbeing check :) How are things progressing with this PR? I was hoping this would make 2.7 but it's looking like it might miss that, and be early 2.8?
Health good. Free time... none. Been on a huge project for work since mid Dec, I'm so over it and would much rather work on this. One week left Mar 22, and I'm out if it's done or not :) then life goes back to normal (whatever that is). But yeah I think it's going to have to be pushed to 2.8.
I didn't make to much progress on companion above where I was before. Got stuck trying to decide how to modify the model/view framework to allow edit the labels. Proxy Model / custom view or what I leaned towards was a custom labels model that connects the main. Made some progress on this one but don't know if I love it.
I think maybe I'll go back to the radio and polish that off first as there wasn't too much else I needed here. At least it can be tested by users for some feedback. Maybe I'll make a separate PR for the companion changes. What do you think?
Ah, that elusive creature, free time. No worries, 2.8 it will be... not like I can crack a whip and make you finish it tomorrow 😁
Yeah, splitting it into two parts will be fine... then the nightly testers will be able to poke and prod the radio side while the companion side is brought up to speed.
Do you still need sample .otx files to test conversions? I've got tons of them.
Do you still need sample .otx files to test conversions? I've got tons of them.
Yeah that would be great. Are you on discord can PM them there?
Also looks like I set this to the side for too long.. Have to see how to incorporate templates now ;)
Also looks like I set this to the side for too long.. Have to see how to incorporate templates now ;)
Yup, that's the danger of stepping away here... resolving and managing the PR conflicts can be fun at times :grin:
I don't think it will disturb the labels UI though, since it hooked into the "new model" path, as well as adding a context menu option to save as a template...
Here is what I have for NV14..
Wish we could do bigger icons, but going to one column looks really bad.
Looking good! Yeah, 1 column would be nice for size, but annoying for having to keep scrolling... maybe we can add a size selector button later down the track if the bottom row doesn't have more stuff that needs to go there.
Back to pretty much where it was before lvgl. Seemed to fix a lot of the scroll issues too and the rtn key issue from before👍
@raphaelcoeffic / @kevinkoenig - I could use some help on these items,
- The labels dialog layout, I still don't quite know what I'm doing wrong here. model_select > ln 484
- Creating a Progress bar for rename from a menu.. The full screen one works but I agree it should be like MPM, tried to imitate it but I'm missing something here. model_select.h > ln 95, model_select.cpp -> ln 678
Overall I think ready for a review minus these 2 items. Let me know, hoping to put this to the masses for testing so I can focus on companion for a while.
This does require the PR # 54 to libopenui
@dlktdr The label dialog should be fine now. For the progress dialog, I just used the same code as I did for the fixed MPM scan dialog, but did not try it out. Just let me know how it goes ;-)
@dlktdr The label dialog should be fine now. For the progress dialog, I just used the same code as I did for the fixed MPM scan dialog, but did not try it out. Just let me know how it goes ;-)
Labels rename looks great 🥇 , down to the last item.. Will look a little more tomorrow too. Doesn't appear paint (progress.cpp) ever gets called for the rename. hmm
Doesn't appear paint (progress.cpp) ever gets called for the rename.
I believe the updateProgress method is not called then ;-)
For some reason I didn't see the state change on this PR... so sorry for the delay there...
There were some conflicts due to the libopenui changes (PR merged btw) and changes to translations, hence the merging main in again so that the automatic builds can run again.
@dlktdr ~~Cliff, can you see if you can manually retrigger the companion builds (windows64, linux, mac) from your end? I think because it's a PR from your repo it needs to be done on your end (I could trip it with a pointless change in companion code but I'd rather not if I can help it 😆 ) ... it would be something like this (but on your fork of the EdgeTX repo) ...~~ Sorry, I didn't realise at the time I said that the companion changes were no longer there (or was that another branch?) 🤦
https://user-images.githubusercontent.com/5500713/181870362-268a4f59-980f-41e4-a32f-c186f0e9ad00.mp4
All good, wasn't too worried as I still have work on companion, Yes it's in another branch. Was looking to have users test this part as it's quite a change to one of the probably the most important things, Opening a model :)
I hope to have a companion draft very soon as I know we need time to test before the Release Candidate.
Hi @dlktdr , I like to merge my #2243 into your branch ? or what do you like ? Also the da.h translation, can be done that way or ? What do you like :-)
Hi @dlktdr , I like to merge my #2243 into your branch ? or what do you like ? Also the da.h translation, can be done that way or ? What do you like :-)
You will probably need to re-work some of it but this works for me. I have sent you an invite so you can modify it.
Just a FYI, as I sort of expected, merging of #1481 has triggered conflicts with this PR, but it looks like it's only minor.
on it :).. Found more issues.... Creating a new model didn't work as expected, found that one. Now duplicating isn't working or delete :)
Should be good again
Are you thinking this is ready to go, and we'll play wack-a-mole as needed? If so, I'll play with this today/tomorrow, with goal of merging in next two days, with followup PRs as needed.
Yeah, I think we're ready to play waccamole. Hopefully it's an easy game ;)
At least getting a review on it would be good.
Just found another bug.. add too many labels and the sort buttons don't stay at the bottom. Put this on the known list.
Just found another bug.. add too many labels and the sort buttons don't stay at the bottom. Put this on the known list.
You should use a grid with the rows defined as:
static const lv_coord_t row_dsc[] = {LV_GRID_FR(1), BUTTONS_HEIGHT, LV_GRID_TEMPLATE_LAST};
Additionally, so that the ListBox is properly constrained, you need to set LV_GRID_ALIGN_STRETCH for the ListBox object.
You could just as well use only one grid for the whole page with:
static const lv_coord_t col_dsc[] = {LABELS_WIDTH, LV_GRID_FR(1), LV_GRID_TEMPLATE_LAST};
static const lv_coord_t row_dsc[] = {LV_GRID_FR(1), BUTTONS_HEIGHT, LV_GRID_TEMPLATE_LAST};
And use row_span = 2 on the ModelsBodyPage.