Not to gang up on Garth here, but the direction arrows on my devices are usually wrong too. Distance is correct but the arrows seem to be pretty random, certainly not correct
I looked and that doesn’t apply. You can do a Google search on “distance and heading from two points” to see it only takes two latitudes and two longitudes. Stationary GPS receivers provide that. It is nothing to do with motion. Here is one of may sites that will do the math for you. Again, this thread is nothing to do with the location of the “N” on the display. What is wrong is that the arrow relative to the N is incorrect.
I looked into it and we can’t remove the copyright notice. It’s correct being there.
The screen.h was initially from another project which is covered by gpl3. Gpl3 indicates that any copyright notices can’t be removed.
That was a bit of fun research
Yes and the N comes from the dead-reckoning based on your movement.
There is a thread here from when the feature was developed, it clearly is using movement Alpha tester thread (1.1.8 device code ready for alpha testing) 🙂 - #134 by CycloMies
If you see a bug in the code that can be corrected pull requests are welcome.
I see that today garth closed Meshtastic defect #1194 that corresponds to this message thread. I’ve explained the defect multiple times and have given replication instructions. I am a retired software Quality Assurance engineer with masters degrees in electrical engineering and computer science. You can trust me. #1194 is a defect. My posts in this message thread explain it.
Garth: Motion of the radio is required to get the “N” indicator in the display pointing correctly. No motion is required to get the direction arrow pointing correctly relative to the “N” and that is what is wrong.
The code is working as expected, and requires motion to produce the N marker on the device.
Pull requests are welcome if you have a better solution in mind, but the code is doing what is expected right now and is motion dependent.
Garth please put defect 1194 back in open status and let someone that understands the problem and the code fix the code. All you are doing is stopping progress on this. It is a defect. The code is not doing what is required by users and is possible. Code pull requests and fixes are the responsibility of the code support people, not those submitting defects.
Please read this slowly and be sure you understand – When you are not moving, the location of the N can be anywhere on the circle. It doesn’t matter where it is. That is not the problem. There is no problem with locating the N on the circle. But once the N is drawn somewhere (anywhere) the arrow indicates the direction of the other radio relative to the N. For example, see my initial post where the N is up-left. No problem. I am not moving and the N can be anywhere. But relative to that N being North the arrow is pointing southwest and should be pointing southeast.
The feature is very clearly based on being in motion, the developer who wrote this code has stated this several times. This feature is not going to be accurate sitting on a table.
Which feature? I would agree that since the device has no compass, you are right that it must be in motion to display “N” correctly. This is one feature. This seems to be, as you say, “working as expected.” Nobody in this thread has argued that it isn’t.
Rick is reporting a defect on a different feature. It is the ARROW drawn within the “compass” circle, and whether it is pointing in the correct direction. Regardless of where the “N” is on the compass (, the arrow ought to be able to display correctly in relation to the “N,” since as Rick pointed out, it is a simple math problem to determine the precise bearing between two known GPS points.
If this is NOT a distinct feature, please correct me. I do not grok code and so I can only bumble my way through the code and read what is written in the docs to try to make my own devices work and be an effective bug reporter.
edit: haven’t figured out this forum’s markup language yet but this was meant to be in reply to Garth’s most recent post in this thread.
The two features are connected. Where are people getting the idea that motion is not necessary? The feature as written works as expected once you walk about 20 meters.
@Rick314 is trying to say that, relative to “N” on the user interface, the direction to the other user is wrong.
Let me try to explain it with crude drawings:
The left part is the ideal case, where the direction of the other node relative to the North is determined correctly. The right part is the buggy case, where the direction of the other node relative to North is determined incorrectly.
Rick is not even talking about “N” being pointed correctly with regards to the device’s physical orientation. That part works well. What he is saying, is that the other node’s relative direction to the “N” on the UI itself is wrong.
Two GPS co-ords are enough to determine the direction between them. The orientation of the device towards North is irrelevant to the issue.
Looks to me like there’s a bug on line 680, where it’s figuring bearingToOther: it passes the two sets of coordinates to GeoCoord::bearing() in the wrong order, so instead of figuring the bearing from you to the other node it really figures the bearing from them to you. So the arrow points backwards from what it should do.
Seems like this would explain the behaviour described by Mihai, which I also am seeing (tbeam firmware version 1.2.52).
Seems unlikely as it would always be wrong if the coordinates were passed in backwards, it would also not include the original issue reported which is essentially that sometimes the bearing is off by ~90 degrees. I don’t have tbeams with screens currently, but I have never seen this work improperly when motion is used and have tested it several times. They code is years old. Put a tbeam at a known heading and then walk 20m or so holding the other device, in my experience it will point where you expect. Put them on a table and they are not going to point in the right direction relative to each other.
OK, did some testing with --setlat/–setlon, and I think I figured out what’s going on. And I am happy to say I think everyone here is correct: there are several bugs, and the ultimate behavior does depend on the travel-heading estimate. I know, it really shouldn’t, but there’s several bugs in there.
Here’s the bugs I found:
- bearingToOther is reversed, as I reported earlier.
- Screen.cpp:681: the arrow direction gets set as (bearingToOther - myHeading), but it should be (bearingToOther + myHeading).
- I’m less confident about this one, but I think everything is drawn with the wrong screen-coordinate system. It’s written as though it uses a standard-orientation x-y system with the origin in the bottom left; but it really uses a reverse-orientation with the origin in the top left. Standard orientation has a rotation of 90deg anticlockwise from +x to +y; reverse orientation would be 90deg clockwise.
Bugs 2 and 3 cancel out if you’re heading close to N or S, which would explain my and probably Mihai’s earlier reports.
Here’s my artificial test cases, and my explanation of what happened according to these three bugs. I told my two nodes they were about 5km apart, with the Other to my NE. Then I told my node it was moving a little bit in various directions. Movement steps were around 300m, and all positions were near 40deg N latitude.
Test 1: move N. Expected N at 12:00, arrow pointing 1:30. Got N at 6:00, arrow at 1:30.
My explanation: estimated heading is 0deg. The N is originally set at 12:00 (Screen.cpp:597-599), gets rotated by 0deg (Screen.cpp:602), and inverted (Screen.cpp:603) to the 6:00 position. Correct bearing is +45deg: bearingToOther is 45+180=+225deg, and subtracting myHeading=0deg makes no change. The arrow is initialized pointing at +y (Screen.cpp:578), which is 6:00, so rotating +225 deg is 1:30, OK.
Test 2: move S. Expected N at 6:00, arrow at 7:30. Got N at 12:00, arrow at 7:30.
Explanation: myHeading is 180deg. The N starts at 12:00, gets rotated 180 to 6:00, and inverted back to 12:00. bearingToOther is +225deg, and the arrow rotation (Screen.cpp:681) is set to 225-180=+45deg. +45deg from +y is 7:30, OK.
Test 3: move E. Expected N at 3:00, arrow at 4:30. Got N at 9:00, arrow at 10:30.
Explanation: myHeading is 90deg. The N starts at 12:00, gets rotated +90deg to 3:00, and inverted to 9:00. Arrow rotation is 225-90=135deg, which is 10:30. OK.
Test 4: move W. Expected N at 9:00, arrow at 10:30. Got N at 3:00, arrow at 4:30.
Explanation: myHeading is 270deg. N starts at 12:00, gets rotated +270deg to 9:00, then inverted to 3:00. Arrow rotation is 225-270=-45deg, which is 4:30. OK
Test 5: move NE (60deg). Expected N at 2:00, arrow at 3:30. Got N at 8:00, arrow at 11:30.
Explanation: myHeading is 45deg. N starts at 12:00, rotates +60deg to 2:00, and inverts to 8:00. Arrow rotation is 225-60=165deg, which is 11:30. OK!
Test 6: move SW (240deg). Expected N at 8:00, arrow at 9:30. Got N at 2:00, arrow at 5-something.
Explanation: myHeading is 240deg. N starts at 12:00, rotates +240 to 8:00, and inverts to 2:00. Arrow rotation is 225-240=-15deg, which is 5:30. OK.
How’s that sound to you all? Did I make some silly mistake? It’s kind of hard to explain without pictures.
If I’m right, bearingToOther definitely needs fixed. I personally think all the myHeading stuff is fragile and finicky and makes the display hard to read, so I’d like to just pull that out and leave the N at the top of the circle. Thoughts?
That is, I think the screen coordinates have (0,0) at top left. +x goes to the right and +y goes down, so positive rotation (+x->+y) is clockwise. The N mark starts at -y (Screen.cpp:599), which is 12:00. The arrow starts out pointing at +y (Screen.cpp:578), which is 6:00.
Did you step through the running code for 1 and 2? It really seems unlikely that this is reversed and then reversed again and causing two different bugs with different behaviors. It is setting the previous position values used in variables so if you are assuming this is a bug based on variable names (p and op) it may be running more times than you are expecting.
I still have never seen it not work when tested with two tbeams and 20 meters of motion to get the heading, did you do this?
Dear juniper, you have made a silly mistake. Line 681 is correct: the wrong-way rotation bug is line 602. Oops.
Yeah, it’s unclear. The two reversals are not happening in the same code path: one reverses bearingToOther, and the other reverses the position of the N marker.
Can you explain what you mean about ``running more times than you are expecting’’? The variables op and p are initialized on each call to drawNodeInfo(). As I understand it, the only relevant variables that are preserved across calls are the three static variables in estimatedHeading(), which are accounted for in my description.
when the bearing function runs Meshtastic-device/GeoCoord.cpp at eaa5252cdb2e6684327de049f59b78c5bdc9144d · meshtastic/Meshtastic-device · GitHub
in the screen code it is also setting oldLat and oldLong values right after it gets the bearing that it looks like are used for the dead reckoning.
That GeoCoord class is somewhat suspicious as it was updated much more recently than the other code involved here
Yes, those are the three static variables in estimatedHeading() that I was talking about. It figures the bearing from (oldLat, oldLon) to the current point and uses that as the current heading estimate. By the way, that call to GeoCoord::bearing() on line 556 shows the correct order of arguments, in contrast to the call on 680.
(That said, this is technically not the correct way to figure heading through two points. I think the really proper way is to take the bearing from the current point back to the previous point, then rotate 180 degrees. Not that the difference is important for this application.)
I think the only GeoCoord routine that we’re using here is GeoCoord::bearing(), right? That one seems pretty transparent, as long as we give it the arguments in the right order.