-
-
Notifications
You must be signed in to change notification settings - Fork 455
feat: increase model limit from 60 to 120 #6208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.11
Are you sure you want to change the base?
Conversation
cddf222 to
2c04c40
Compare
|
@pfeerick I was able to test the windows version and it worked great! However, as I was testing, I was able to discover a couple bugs on the radio firmware. I'll submit those now. |
|
Added an issue here. Hope this is the proper process as I haven't submitted a bug against a pull request before. #6213 |
|
A mix of 2 and 3 digit name suffixes could also trigger more issues eg sorting |
|
It doesn't really matter, but it is probably better to annotate an open PR if there are "bugs" in it/resulting from it (since they are not relevant to the main code base) rather than create an new issue... probably more relevant here as this PR is more an experiment and to ease getting hold of companion builds. Regardless, thank you for the feedback and testing it out... good to know it "sort of works" and where it breaks ;) |
|
@elecpower I assume the solution would be to structure the name with 3 digits and padding: Model001, Model011, Model100, Model255, etc so we'd always have the same number of digits. |
|
That would avoid sorting/sequencing problems. I have a nagging thought of some regex filters existing in the code that may only accept 2 digits. |
@elecpower this is the only file I could find that appears to be handling model naming, but it's in the companion folder. Any other thoughts for where I can poke around? Looking for how the radio names new models "model01.yaml" files it creates on the SD card. edgetx/companion/src/firmwares/modeldata.cpp Line 235 in bf1ef5c
|
|
In Companion read models and settings from radio and sd card may use a filter. |
|
Color radios name the model files without leading 0's in the number (model1, model10.yml, model100.yml etc). B&W radio code is hard wired to generate model file names with a two digit number in the model name and start from model00.yml. Maybe limit the change to 100 models for B&W radios (for now). |
|
No, for the specific application that triggered this more than 100 is needed on B&W, so is not really an option. If this PR was to be something that is actually merged, sure, limiting to 99/100 models will probably work just great, and there is probably no real reason not to bump it to that (and uniformally - rather than be the same but different for three 'categories' of radio :-/). But this is also helping in finding where some parts of the code are still quite brittle / limited by certain assumptions, so is worth pursing further since someone is interested in making it happen. |
|
Extending B&W to handle > 100 will be complex. |
@philmoz what if we don’t extend the padding? Ex: model00, model01, model10, model100, model120, etc I mention that because if I use the companion app in this PR, I’m able to create models at slots 105 and 120 with no issue and the radio can read and interact with them just fine (reads models named model105, model120, etc). Based on my investigation thus far, the issue might be limited to generating new models and moving models into this slot ranges from the radio, as noted in the issue I posted. #6213 If you happen to know which part of the code for B&W radios controls naming new model files on the SD card, I can try some experiments. |
|
what if we don’t extend the padding
This is what I was thinking also, as there is no "need" to alter the naming
of the old models, only work with the new ones. But probably no reason it
can't be an automatic startup rename if the filename name is matched to be
one character short and right filename structure if that was needed. Then
there is no "strange" handling that only kicks in only some of the time.
…On Fri, 9 May 2025, 6:38 am gfalgiano, ***@***.***> wrote:
*gfalgiano* left a comment (EdgeTX/edgetx#6208)
<#6208 (comment)>
Extending B&W to handle > 100 will be complex. It will also need to able
to handle/convert all existing model files in both radio and companion.
@philmoz <https://github.com/philmoz> what if we don’t extend the padding?
Ex: model00, model01, model10, model100, model120, etc
I mention that because if I use the companion app in this PR, I’m able to
create models at slots 105 and 120 with no issue and the radio can read and
interact with them just fine (reads models named model105, model120, etc).
Based on my investigation thus far, the issue might be limited to
generating new models and moving models into this slot ranges from the
radio, as noted in the issue I posted. #6213
<#6213>
If you happen to know which part of the code for B&W radios controls
naming new model files on the SD card, I can try some experiments.
—
Reply to this email directly, view it on GitHub
<#6208 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJ66KMGYPJQWKF3TYGPLEL25O6DTAVCNFSM6AAAAAB4QRJ4GSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNRUGI2TEOBYGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@pfeerick I found the sections we need to change. edgetx/radio/src/storage/sdcard_yaml.cpp One section of the code controls how the models are named on the SD card. It was limiting the number at the end to two digits. Once I changed that, I found a similar issue when you try to back up one of the models over 100. I made another change to address that. Finally, I corrected an issue where it would ignore small model names such as "a" and incorrectly use the model number as the backup file name. With those fixed, the only remaining issue I've seen is with the * next to the selected model. It's one charter overlapping when a model over 100 is selected on a B&W radio. It's minor, so I thought the GUI didn't need to be touched at this time. |
d5e9585 to
af2c6c5
Compare
Summary of changes: