BFCOMPAT mods to increase precision in value scalings #8866
Conversation
|
Just for some background, similar approach has been done for some critical fields in the past, but we kept those to a minimum to make the changes less intrusive and keep things simple as we still have some rays of hope we will be able to drop this If we are going to add special cases for BF all over I think I would prefer to just bump the field sizes for all cases or add a configuration parameter for field length as just increasing the field length will have a negative impacts on analog, where the grid is a lot smaller. I will let others weight in on this change. |
Yes, I agree that changes should be kept to minimum. And to not affect the already space-constrained analog OSD, the BFCOMPAT mode checks are there, so the field sizes will only be bumped if that mode is active. Otherwise, it's just as before. However, making the field sizes a configurable option would be a more flexible and probably better "permanent" solution, specially if things start pointing in the way that DJI won't implement full MSP support on their side... |
|
I was able to put in some test flights yesterday. Almost everything worked as expected, except for the distance field. Below 100 meters, it would display an extra, fixed, digit. So, for example, if 75 meters away from home, it would display 752. If 80 meters away, it would display 802 and so on. Probably something not working as expected inside of osdFormatCentiNumber(), but instead of trying to fiddle further with it, I decided to add a simplified and non-scaling function for it's formatting, very similar to what was done with the altitude field. So distance is only displayed in either meters or feet, without scaling to km or miles, 1500 meters is displayed as 1500 meters, not as 1.50km. I believe this is an acceptable compromise. These changes are in 5c6b682. I haven't had the chance to test these latest mods in flight, but I believe we have gotten to a point where this is ready to be merged, so I will remove the draft status. Still, every feedback is always welcome. :-) |
You can probably use/extend src/test/unit/osd_unittest.cc to help you chase the reason why the formatting broke with the extra digit. Scaling of distance may be more important than altitude though. You are more likely to be out at 1km+ than at 1km up. The scaling is there so that we don't run out digits. |
I can try running those unit tests, but in any case, I have made the distance field 5 digits long, so even without scaling, it will only run out of digits at 100+km. I don't think many people are flying to those kind of distances... ;) |
|
Thanks a lot for the suggestion to use the unit test file to chase the formatting errors. I was able to track down the reason due to insufficient leading space insertion when more than 1 leading space was needed. Rather simple fix, and as a bonus, it should fix the scaling for the altitude fields too. So I have modded the osd.c file to use the formatting functions that apply scaling, and removed the "simple" functions (actually, just commented them out for now) for both the distance and altitude values. To keep things even simpler, since the functionality under BFCOMPAT mode is essentially the same, both the altitude and distance fields use the same formatting function (under bfcompat mode). All these mods are in c9a2c2c. |
|
@DzikuVx should this be targeted to 6.0 or 6.1? |
|
I'd say 6.1 |
When using BFCOMPAT OSD mode, with the decimal point taking one extra character, the decimal scaling feature of certain values does not work as expected. In the case of altitude or home distance, when it goes over 999meters, then the field would change to 1.00km. But what happens in BFCOMPAT mode is that it changes to 0.1km when over 99 meters, thus resulting in 100 meter steps (0.1, 0.2, 0.3 and so on), which is not ideal.
Upon investigating, it seems the "problem" is on the calls to osdFormatCentiNumber() with a fixed number of digits (3). Since the decimal point is considered a digit in itself, it leaves only 2 digits for the actual values. So this PR increases the number of digits in those calls when BFCOMPAT mode is active, so that the 3 value digits are maintained. Downside is extra space used on the screen to have one extra character/digit field.
So far, I have implemented these changes for:
For the altitude field, the change in #8779 seems to "fix" this problem, so I haven't changed that.
With all that said, I have only tested this code on the bench for any obvious errors, I will not be able to flight test (to see if the values are displayed as expected) until the weekend, so if anyone is able to test it sooner and give feedback it will be greatly appreciated! Also any other suggestions/feedback is welcome.