-
-
Notifications
You must be signed in to change notification settings - Fork 361
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
Fix YAML write issue with mix offset #1743
Conversation
@raphaelcoeffic Thank you very much. I merged this PR in a private build. This fixes the "offest with GV's" problem I demonstrated in GV1YAML1.otx.zip and with my real 2.3.14 model .bin files. I didn't notice anything else out of the ordinary but mind you this was very limited testing. So I can't say if this PR breaks anything else. I hope YAML conversion gets more and broader re-testing. Michael |
@raphaelcoeffic - Hi Raphael, a nearly totally unrelated question. Why does destCH counting start at 0? My guess is one reason for switching to YAML was to have model definitions in a human readable format. Looking through the .yml files I found myself getting confused with destCH over and over again. Not that I am not used to indexes starting at 0 but in this case: the channel numbering convention for human users start at 1. Not easy to keep this in mind looking through .yml files not to speak of the mental trap you are running in editing .yml by hand. I think same goes for Inputs Ix. |
This would be the unfortunate "programmer vs human" dilemma... i.e. counting always starts from 0, not 1. Being human readable was certainly one reason for the change to yaml, but the underlying reason was to have it something that was not locked in a binary structure file... ie. the config format itself now does not place any limiations on the sizes of fields... if we want to make it so you can have 30 character model names... we can do that now... without it crashing into the next field in the file. So whether the indexing is changed in a future version I don't know... it depends how far we want to go with making it "human readable"... as if you wanted to start on that aspect, there are a lot more settings that are currently complete gibberish to interpret unless you look at the code... But, IMO, at the end of the day, we don't really want you to be looking at the yaml... that's for the radio or some companion software... if you're looking at it, you're either a programmer working on the firmware, or a companion tool of some sorts, or an advanced user. If you're not one of those... we've failed ;) |
Yeah, I get that. But human readable files are click bait for curious people like me :-) even if they shouldn't :-). And I fully understand the "we don't want you". Your honor, I rest my case. Thanks for your time! |
Ahem... what did I say... I forget now... it must have been good though! 😆 I'm afflicted by that same complaint though... if files are half-way intelligible.... the temptation to play around in them, and see what Easter eggs or features missing UI there are is just irresistible. Anyway, as far as this PR addressing the main issue, It seems to be resolving it without any other side effects... I did a crazy model with all sorts of positive and negative offsets, and weights, -GV9/GV9, GV1/-GV1, (and opposites), basically anywhere I could see that accepted GV I shoved values in at both ends of range for inputs, mixes and outputs, and it seemed to behave itself... I did find out I had to use firmware version 2.5 to test one part of this (Mixer line, Curve Diff GV) as Companion 2.5 did not seem to be generating correct model bins for the radio - neither radio nor simulator liked curve diff with any GV value... mixer rows would show up empty... but was fine if created on radio - companion would read it, and firmware would correctly convert with this PR... companion 2.5 must have been broken for that particular setting. |
Thanks for testing. My worries were about the other places GV's are used. Great you covered this too. My primary use case btw was "I'm migrating to Edgetx" using Opentx 2.3.14 generated .bin's. |
Thanks Raphael & Peter |
Fixes #1734
Summary of changes: