search

anmpout / osmdroid

Automatically exported from code.google.com/p/osmdroid
0 stars 0 forks source link

MyLocationOverlay direction arrow points in the wrong direction #241

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Expected behavior: Arrow should show the orientation of the phone when using 
the compass, and the direction of motion when using the gps bearing.

Current behavior: Arrow is pointing east when it should be pointing west and 
vice versa. (When using the compass, the arrow always points to actual north, 
like the compass rose.)

Fix: Change line 287 from:

                this.directionRotater.setRotate(-bearing + azimuthRotationOffset,
to:
                this.directionRotater.setRotate(bearing + azimuthRotationOffset,

Original issue reported on code.google.com by toreleve...@gmail.com on 15 Jul 2011 at 11:35

GoogleCodeExporter commented 9 years ago 
Which version are you using?

This is probably the issue that was fixed in revision 943.

Original comment by neilboyd on 15 Jul 2011 at 2:25

GoogleCodeExporter commented 9 years ago 
I have tested 3.0.4 and the current trunk (the fix refers to the trunk). 
Revision 943 as far as I can tell fixes line 316 but messes up line 287. 
Direction arrow and compass rose should rotate in opposite directions!

Original comment by toreleve...@gmail.com on 15 Jul 2011 at 3:22

GoogleCodeExporter commented 9 years ago 
It was actually revision 321 that broke this. It changed a simple

if (mLocation.hasSpeed() && mLocation.getSpeed() > 1)

into a whole load of stuff about the compass.

I think it should be simple:
if there's a bearing show the arrow pointing in the direction of the bearing,
otherwise show the man.

Perhaps there should be a third condition:
if there's a bearing show the arrow pointing in the direction of the bearing,
else if there's a compass bearing show the arrow pointing in the direction of 
the compass,
otherwise show the man.

Attached is a patch for the first variation.

Original comment by neilboyd on 19 Jul 2011 at 7:27

Attachments:

GoogleCodeExporter commented 9 years ago 
It would probably also be a good idea to refactor this as described in the 
Google MyLocationOverlay docs:

http://code.google.com/android/add-ons/google-apis/reference/com/google/android/
maps/MyLocationOverlay.html#draw(android.graphics.Canvas, 
com.google.android.maps.MapView, boolean, long)

calls through to drawMyLocation if we have a location fix, and then to 
drawCompass if we have a compass reading

Original comment by neilboyd on 19 Jul 2011 at 10:15

GoogleCodeExporter commented 9 years ago 
For my app it makes a lot of sense for the direction arrow to show what 
direction the phone is pointing, more so than to show the GPS bearing, but I 
guess that depends on the app.

However, if you implement getOrientation() from the Google MyLocationOverlay 
(or make mAzimuth protected) I could easily subclass draw() to do my own 
drawing. Subclassing drawMyLocation() and drawCompass() would be even nicer. 
But most important is to make compass orientation accessible from subclass.

Original comment by toreleve...@gmail.com on 19 Jul 2011 at 11:48

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
I've attached another patch which adds getOrientation() and refactors draw() 
into drawMyLocation() and drawCompass().

Original comment by neilboyd on 19 Jul 2011 at 3:50

Attachments:

GoogleCodeExporter commented 9 years ago 
I have now tried the patched version. It seems to work mostly as expected. A 
couple of issues though:

When the compass is enabled but mAzimuth has not been set, an empty compass 
frame is drawn. The problem is "mAzimuth != Float.NaN" always evaluates to 
true. Use "!Float.isNaN(mAzimuth)" instead.

I'm not sure how Google does this, but in draw() you might want to call 
"drawCompass(canvas, mAzimuth)" instead, and move the azimuthRotationOffset 
stuff to drawCompass().

Also, you might want to move the pj.toMapPixels stuff from draw() to 
drawMyLocation() since it only serves to set mMapCoords which is then used in 
drawMyLocation().

I have tried overriding drawMyLocation() with my own drawing method (using 
getOrientation()) and it works well, except it would be nice to have protected 
access to DIRECTION_ARROW_CENTER_X and DIRECTION_ARROW_CENTER_Y.

I have not tried to override drawCompass() but it would probably be nice to 
have protected access to COMPASS_FRAME_CENTER_X, COMPASS_FRAME_CENTER_Y, 
COMPASS_ROSE_CENTER_X and COMPASS_ROSE_CENTER_Y as well.

Original comment by toreleve...@gmail.com on 19 Jul 2011 at 8:30

GoogleCodeExporter commented 9 years ago 
I committed this in revision 961 including some of your comments.

I'm not sure whether getOrientation is supposed to include the adjustment for 
device rotation or not. I don't know how it's supposed to be used by subclasses.

There's a big comment in getOrientation about device rotation, which points to 
an error that I can see on my phone, so I'll see if I can fix that next.

Original comment by neilboyd on 20 Jul 2011 at 8:40

GoogleCodeExporter commented 9 years ago 
I fixed the device rotation in revision 962.

Original comment by neilboyd on 20 Jul 2011 at 9:05

GoogleCodeExporter commented 9 years ago 
I think I'm finished with this now. Revision 964.

Original comment by neilboyd on 21 Jul 2011 at 5:27

GoogleCodeExporter commented 9 years ago 
Looks good, except line 231 needs to be moved so that it is run even when 
mDrawAccuracyEnabled is set to false. Otherwise line 270 won't work correctly. 
(I would also change "mGeoPoint" to "myLocation" on line 231.)

Original comment by toreleve...@gmail.com on 21 Jul 2011 at 9:50

GoogleCodeExporter commented 9 years ago 
I've done exactly that in revision 966.

Original comment by neilboyd on 21 Jul 2011 at 11:22