Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simplify map rotation and use a low pass filter for damping #982

Closed
wants to merge 3 commits into from
Closed

simplify map rotation and use a low pass filter for damping #982

wants to merge 3 commits into from

Conversation

hochwasser
Copy link

@hochwasser hochwasser commented Mar 19, 2018

This PR will close #636
The following issues are addressed:

  • rotation of the map in the direction one is looking
  • suppress nervous map rotation

@rugk
Copy link
Contributor

rugk commented Mar 19, 2018

You can (automatically) let issues close when a PR is merged, i.e. to make the process straightforward, you may add the "magic text" there in the pull request description.

/*
* time smoothing constant for low-pass filter 0 ≤ alpha ≤ 1 ; a smaller
* value basically means more smoothing See: http://en.wikipedia.org/wiki
* /Low-pass_filter#Discrete-time_realization
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not split URL in two lines

@hochwasser
Copy link
Author

I changed the file and it seems to have worked to push it to github.
Is there something else what needs to be changed?
Has someone tested if the map get's rotated right?

@rugk
Copy link
Contributor

rugk commented Mar 21, 2018

I hope you tested it.😉
But I'm sure @westnordost will soon review your PR and test it. Just stay patient…😉

@hochwasser
Copy link
Author

I tested it :-)
But every android device has some quirks and in this respect is unpredictable. :-(

Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit, I do not really understand how/why this works (if it works, I did not test it yet), so I only commented on some code style issues.

Did you test that it works when:

  • Tilting the phone?
  • Rotating the phone from portrait mode to landscape mode, upside down etc.?

{
if(compassTimer == null) initializeCompassAnimator();
float Declination = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable names should start with lowercase


compassAnimator.targetRotation = azimut + displayRotation;
compassAnimator.targetTilt = displayTilt;
compassAnimator.targetTilt = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

targetTilt is always 0? This can't be correct,

private LowPassFilter filterYaw = new LowPassFilter(0.03f);

static float rot = (float) 0.0;
static float tilt = (float) 0.0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no static mutable fields here.

@westnordost
Copy link
Member

Also, CompassAnimator does some smoothing on its own. So, this could be removed with this low-pass filter in place?

@ENT8R
Copy link
Contributor

ENT8R commented Apr 23, 2018

@hochwasser will you continue to work on this PR or did you forgot it?

@hochwasser
Copy link
Author

@ENT8R I'm working on it. Before I do the next move the open issues from westnordost have to be fixed.
The first thing I noticed: the google android emulator is in the acceleration, magnetic and rotation sensor not correct. The devices I tested behave completely different.
I looked at the implementation of satstat, they have a sensor tab and this seems to be correct for the devices I tested (checked with an actual compass).
TYPE_ORIENTATION is deprecated, but with the new way of getting the device orientation I can't see any correlation to the real orientation of the device.

The next thing in line is the interface to the map. Then I have to figure out what use "tilt" has.

@ENT8R did you do anything in the map rotation area?

@ENT8R
Copy link
Contributor

ENT8R commented Apr 24, 2018

@ENT8R did you do anything in the map rotation area?

I actually tried to implement a very first version of the automatic map rotation with #436 some time ago

the google android emulator is in the acceleration, magnetic and rotation sensor not correct.

Why is it not correct?

Then I have to figure out what use "tilt" has.

Google Maps does it this way. In StreetComplete you have to push up with two fingers to achieve the same effect.

@westnordost
Copy link
Member

I implemented something like this now myself.

@ENT8R
Copy link
Contributor

ENT8R commented May 1, 2018

I just tested it but from my point of view this filters out a little bit too much... The map is only moving very slow. And what is strange in addition to that is that the blue triangle which is showing the direction is moving much more faster than the map.

@westnordost
Copy link
Member

The smoothing is a big extreme, yes, but perhaps you have a good sensor in your smartphone. There are a lot of shitty sensors out there. Also, now it behaves like a good-old analog compass.

This can't be. I didn't change anything in that regard.

@ENT8R
Copy link
Contributor

ENT8R commented May 1, 2018

This can't be. I didn't change anything in that regard.

I mean the map uses a filter now but the blue direction triangle is still using no filter so the movement is different...

@westnordost
Copy link
Member

No, the direction triangle also uses that filter

@hochwasser
Copy link
Author

@ENT8R
My Problem with the emulator was that the values I got from the API were not close to the values from the real device when I rotated the virtual/real device.
This is still strange to me. So I had to test it outside with the real device.

@hochwasser hochwasser deleted the pulls/rotate_map branch May 4, 2018 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Map rotates nervously in compass mode
5 participants