AGL shifting by ~1 point

[Jenatko]

Hi, i believe the algorithm for determining agl from coordinates gives slightly errorous results.

agl.cpp, this section:

    // "-2" is, because a file has a overlap of 1 point to the next file.
    uint32_t coord_div_x = GPS_COORD_MUL / (num_points_x - 2);
    uint32_t coord_div_y = GPS_COORD_MUL / (num_points_y - 2);
    uint16_t y = (lat % GPS_COORD_MUL) / coord_div_y;
    uint16_t x = (lon % GPS_COORD_MUL) / coord_div_x;

works kinda okay considering its only int arithmetic, but due to truncation of decimal points in coord_div_x/y, there is an error for coordinates with high decimal fraction.

example for num_points=1201 and lon = 49.9998999° (499998999 as represented by integer):

coord_div_x = 1e7/(1201-2) = 8340,28 -> 8340 as an int
x = (499998999 % 1e7) / 8430 = 9998999 / 8430 = 1198.92 ->1198 as integer

so the agl values will be interpolated from somewhere between column index 1198 and 1199, whereas the correct result is between 1199 and 1200. (0.9998999° is 0°59'59.64'')

This error should be eliminated by coord_div_x = GPS_COORD_MUL / (num_points_x - 1), but then using float is required. integer arithmetic could result in trying to access 1202th point (which doesnt exist)

[bubeck]

Thanks for your feedback, that I just saw recently. Sorry for not answering sooner, as I was busy doing other things.

I started with float arithmetic and then changed to int introducing the above problem. You are absolutely right with your finding. I was always wondering, why "-2" really was needed, now I understand better.

I am an open source entuthiast and not affiliated to skybean in any way. As SkyDrop is only receiving fixes needed for slightly newer components, I dont think that this fix will make it into the code, as the risk is probably considered too high. Changing to float would probably be not such a big speed penalty...

Thanks again!

[Jenatko]

Hi, this is such a delayed reply i don't even remember what it is about :-D

---

Od: "Dr. Tilmann Bubeck" @.> Komu: "fhorinek/SkyDrop" @.> Datum: 05.06.2024 09:57 Předmět: Re: [fhorinek/SkyDrop] AGL shifting by ~1 point (#443)

CC: @.>   Thanks for your feedback, that I just saw recently. Sorry for not answering sooner, as I was busy doing other things. I started with float arithmetic and then changed to int introducing the above problem. You are absolutely right with your finding. I was always wondering, why "-2" really was needed, now I understand better. I am an open source entuthiast and not affiliated to skybean in any way. As SkyDrop is only receiving fixes needed for slightly newer components, I dont think that this fix will make it into the code, as the risk is probably considered too high. Changing to float would probably be not such a big speed penalty... Thanks again! — Reply to this email directly, view it on GitHub https://github.com/fhorinek/SkyDrop/issues/443#issuecomment-2149131405, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMKTXC2QACHAQKGUWVZKJOLZF3AC3AVCNFSM4ZFRMX72U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJUHEYTGMJUGA2Q. You are receiving this because you authored the thread.Message ID: @.>