FrSkyRC / ETHOS-Feedback-Community

Feedback & suggestions are welcomed here for ETHOS by FrSky
189 stars 85 forks source link

Moving / deleting of VARs can change VAR in use in mixer(s?) #3213

Closed Jedsters closed 9 months ago

Jedsters commented 9 months ago

Moving / deleting of VARs can change VAR in use in mixer(s?)

To recreate:

Create a basic model with a motor Add 3 VARs called e.g. VAR1, VAR2, VAR3 in that order Add a Throttle Mixer Under Throttle Cut give it a Trigger Value using a Source of VARs and select VAR2 Now return to VARs and move VAR3 to the top of the list so the order is now VAR3, VAR1, VAR2 Return to Throttle Cut, Trigger value and you will see that it is now using VAR1! I have not checked for other occurences either in other mixers or other areas where moving of VARs could result in the VAR selected incorrectly being changed.

This feels like exactly the same problem as I reported both in #3199 and #1249, albeit that they were for curves and FM respectively. I am guessing that when items (FM, Curves, VARs or the next thing to be introduced which has a list particularly which can be re-ordered) are reordered for any reason, including deletion of items, that you then have find every occurence where these are used and make sure the code updates every occurence with the every new positions in the list. The problem comes when you either fail to find one or it seems for this and #3199 when you introduce a new feature and it doesn't get included into the list of items which may need renumbering. If this is correct, then I can't help feel that we are going to have more issues like this and unfortunately these tend to result in potentially very significant changes to how a model behaves which may not be immediately obvious to a user. I'm sure very few users will consider that they need to thoroughly validate how a model works if they just change the order of a list or delete an unused item in a list.

If I am correct, then surely an approach which involves indexing is more appropriate so that when a list is reordered only the index needs to be updated and that will automatically take care of every reference to items in that list, be that in current code or future code. Apologies if I am sticking my nose in where it isn't wanted, but it is concerning having raised 3 issues for essentially the same problem.

Thanks

bsongis-frsky commented 9 months ago

The throttle mix didn't declare to the visitor some of the sources he was using (cut.triggerValue, cut.idleValue, hold.value) The fix is ready for RC1

bsongis-frsky commented 9 months ago

@Jedsters I have re-enabled the nightly builds for 1.5.0 until RC1 comes. Only the bugs found in RC0 are fixed (no new feature). So you can follow the progress everyday.