Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

GH-337 Android Low Pass Filter #354

Merged
merged 9 commits into from
Jul 17, 2018
Merged

Conversation

Mrnikbobjeff
Copy link
Contributor

@Mrnikbobjeff Mrnikbobjeff commented Jul 6, 2018

Description of Change

Compass api extended by ApplyLowPassFilter property. Internally it uses a moving average filter. No unit tests were added, as there were no Platform specific unit tests.

Bugs Fixed

None

API Changes

Added:

  • bool - Compass.ApplyLowPassFilter { get; set; }

Changed:
None

Behavioral Changes

Low pass filter is initially disabled by default. @Redth suggested applying it by default, but this would be a behavioural change

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation

@msftclas
Copy link

msftclas commented Jul 6, 2018

CLA assistant check
All CLA requirements met.

@dend
Copy link
Contributor

dend commented Jul 6, 2018

⚠️ Validation status: warnings

File Status Preview URL Details
⚠️Warning Details

  • Line 80: [Warning] Failed to run script W:\kdto-s_dependentPackages\ECMA2Yaml\tools\Run.ps1 with exit code -1

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@dend
Copy link
Contributor

dend commented Jul 6, 2018

⚠️ Validation status: warnings

File Status Preview URL Details
⚠️Warning Details

  • Line 80: [Warning] Failed to run script W:\c1in-s_dependentPackages\ECMA2Yaml\tools\Run.ps1 with exit code -1

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@dend
Copy link
Contributor

dend commented Jul 6, 2018

⚠️ Validation status: warnings

File Status Preview URL Details
⚠️Warning Details

  • Line 80: [Warning] Failed to run script W:\eyhq-s_dependentPackages\ECMA2Yaml\tools\Run.ps1 with exit code -1

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@jamesmontemagno jamesmontemagno changed the title Feature/issue337 GH-337 Android Low Pass Filter Jul 6, 2018
Copy link
Collaborator

@jamesmontemagno jamesmontemagno left a comment

Choose a reason for hiding this comment

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

A little comment and also can you add the new property to these: https://github.com/xamarin/Essentials/tree/master/docs/en/FrameworksIndex

@@ -52,8 +53,43 @@ internal static void PlatformStop()
}
}

class LowPassFilter
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that since this is generic it could be moved over to the .shared.cs file perhaps.

@Mrnikbobjeff
Copy link
Contributor Author

Mrnikbobjeff commented Jul 8, 2018

@jamesmontemagno I adressed the changes. everything is done as you asked

@dend
Copy link
Contributor

dend commented Jul 8, 2018

✅ Validation status: passed

File Status Preview URL Details
Samples/Samples/View/CompassPage.xaml ✅Succeeded
Samples/Samples/ViewModel/CompassViewModel.cs ✅Succeeded
Xamarin.Essentials/Compass/Compass.android.cs ✅Succeeded
Xamarin.Essentials/Compass/Compass.shared.cs ✅Succeeded
Xamarin.Essentials/Compass/LowPassFilter.shared.cs ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-android.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-ios.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-uwp.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials.xml ✅Succeeded
docs/en/Xamarin.Essentials/Compass.xml ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@dend
Copy link
Contributor

dend commented Jul 8, 2018

✅ Validation status: passed

File Status Preview URL Details
Samples/Samples/View/CompassPage.xaml ✅Succeeded
Samples/Samples/ViewModel/CompassViewModel.cs ✅Succeeded
Xamarin.Essentials/Compass/Compass.android.cs ✅Succeeded
Xamarin.Essentials/Compass/Compass.shared.cs ✅Succeeded
Xamarin.Essentials/Compass/LowPassFilter.shared.cs ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-android.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-ios.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-uwp.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials.xml ✅Succeeded
docs/en/Xamarin.Essentials/Compass.xml ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@Mrnikbobjeff Mrnikbobjeff mentioned this pull request Jul 9, 2018
@Redth Redth requested a review from charlespetzold July 16, 2018 15:29
@charlespetzold
Copy link
Contributor

The LowPassFilter class essentially averages the last 10 (or up to 10) readings. The naïve approach would involve simply saving and averaging the last 10 angle values. But this can easily lead to serious errors. For example, suppose the last 10 readings were (in degrees) 5, 355, 5, 355, 5, 355, 5, 355, 5, 355. The average is 180, which is completely wrong. The average should be 0 or 360.

This problem occurs whether you're working with degrees or radians, and whether the raw readings range from 0 to 360, or from -180 to 180. In the second case, if the 10 readings (in degrees) are -175, 175, -175, 175, -175, 175, -175, 175, -175, 175, the average is 0, which is also completely wrong. It should be -180 or 180.

The clever solution in LowPassFilter is to instead average the sine and cosine values of the angles, which wrap much better throughout the range of angles.

For the 10 values that bounce between 5 and 355, the sine of 5 degrees is 0.087... and the sine of 355 is -0.087... The sine values average to 0. The cosines of 5 and 355 degrees are both 0.996... so the average is also 0.996... The value of Math.Atan2(0, 0.996...) is not exactly 0, but it is very close. (The method returns -4E-15.)

For the 10 values that bounce between -175 and 175, the sines are 0.087 and -0.087, averaging to 0. The cosines are both -0.996, averaging to -0.996..., and the value of Math.Atan2(0, -0.996) is very close to 180.

I think the miniscule errors introduced by LowPassFilter are inconsequential -- particularly considering the simplicity and elegance of the code.

@dend
Copy link
Contributor

dend commented Jul 16, 2018

✅ Validation status: passed

File Status Preview URL Details
Samples/Samples/View/CompassPage.xaml ✅Succeeded
Samples/Samples/ViewModel/CompassViewModel.cs ✅Succeeded
Xamarin.Essentials/Compass/Compass.android.cs ✅Succeeded
Xamarin.Essentials/Compass/Compass.shared.cs ✅Succeeded
Xamarin.Essentials/Compass/LowPassFilter.shared.cs ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-android.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-ios.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials-uwp.xml ✅Succeeded
docs/en/FrameworksIndex/xamarin-essentials.xml ✅Succeeded
docs/en/Xamarin.Essentials/Compass.xml ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@Redth Redth merged commit 56bc8dc into xamarin:master Jul 17, 2018
@jamesmontemagno jamesmontemagno added this to the 0.9.0-preview milestone Jul 19, 2018
jamesmontemagno added a commit that referenced this pull request Jul 31, 2018
* GH-343: Take into consideration VerticalAccuracy on iOS Altitude (#344)

* Fix for #343

Only set altitude if it's available (VerticalAccuracy is negative when it isn't according to iOS doc)

* Use: (double?)

* dotnet foundation CLA broken link (#349)

* Update ns-Xamarin.Essentials.xml

* Update README.md (#340)

* GH-365 Change to generic EventHandlers

* Clean up unit tests.

* GH-363 Enable C# 7.2 (#364)

* GH-363 Enable CD 7.3

* Update Xamarin.Essentials.csproj

* GH-367 Change Ui to UI & Ac to AC (#374)

* GH-367 Change Ui to UI

* Ensure proper Ui for Android

* Fix gryoscope

* Rename Ac to AC

* Update framework indexes

* Create Product Feedback page for Docs (#369)

* Create PRODUCT-FEEDBACK.md

* Update PRODUCT-FEEDBACK.md

* Fixed a typo from "chanaged" to "changed"

* Fixed a typo from "chanaged" to "changed"

* Fixed a typo from "chanaged" to "changed"

* GH-337 Android Low Pass Filter (#354)

* Implemented low pass filter as requested by issue 337

* Added value to sample page.

* Updated docs for ApplyLowPassFilter

* Enabled ApplyLowPassFilter Switch on Android, disabled on all other platforms

* Moved LowPassFilter to shared, removed unnecessary usings.

* Updated Docs

* Sample Project Tweaks and Optimizations (#353)

* Make sure orientation sensor stops on sample.

* Fix up all sensors page to start and stop correctly

* Only check permission if unknown status

* Update to latest sdk.extras

* fix back button press on TTS page. Check for null.

* Add samples sln for CI/CD

* Update readme and samples sln

* Don't use code sign key for simbuilds

* let's try this gain

* Update to projects

* Cleanup sample config

* bump sdk extras

* Add in readme for install with helpful information. Bump MSBuild.Sdk.Extras (#392)

* GH-368: Allow for unicode in host names and escape query (#393)

* Allow for unicode in host names and escape query

Previously we were passing the the url as is to each platform and letting it deal with how to parse into the platform specific url.

This handles unicode domain names (IDN Mapping aka PunyCode) for the domain, but also reconstructs the url based on the parsed System.Uri from the original string, and so the PathAndQuery will be the % escaped version which will account for unicode characters in it as well.

Finally, on iOS and Android we are using AbsoluteUri to end up with the fully IDN mapped and % encoded URL to pass to the system URL types.

* Extract Escape Uri logic into its own method

More testable.

* Add uri escaping tests

* Improve uri escape tests

* Update default iOS DeviceTests target

Also don’t allow picking a sim that’s unavailable (this was a bug since we were checking for contains `available` which of course `unavailable` also contains!)

* Update iOS DeviceInfo.Model to return specifid hw.machine for more control. (#395)

via:
https://github.com/xamarin/xamarin-macios/blob/bc492585d137d8c3d3a2ffc827db3cdaae3cc869/tests/linker/ios/link%20sdk/LinkSdkRegressionTest.cs#L603-L631

Added tests

* GH-380 Use Internal Preferences for Secure Storage consistency. (#386)

* Use Internal Preferences for Secure Storage consistency.
Save if we created key pre-M so we always use pre-M if device upgrades.

* Fix logic for pre-m key check

The logic was slightly off, I think this fixes it, but would be good to have another set of eyes...

1. We check to see if the device is pre-M (if it does _not_ have `M`, or if we already set the fact it's pre-M in the preference - aka from a previous install before an upgrade of the OS)
2. If we aren't pre-M, we can't use Symmetric Key from Keystore
3. If we make it down to using Asymmetric Key, we set the pre-M preference to `true` to persist the value for future invocations, which will make it 'stick' in the event of a pre-M to M+ OS upgrade.

* Address feedback on key naming.

* Added test for secure storage to simulate upgrade

From API < 23 to API >= 23 after storing data with an asymmetric key and then moving to a platform supporting symmetric keys.

* Ensure we always set flags when using specified keygen

* GH-388 If activity is required and null throw null exception with information. (#396)

* If activity is required and null throw null exception with information.

* Remove currentactivity and update getcurrentactivity.

* GH-250 Implement Launcher API (#356) (#405)

* GH-250 Implement Launcher API (#356)

* Initial commit for Launcher Api as discussed in Issue 250

* Fixed and refactored android launch code, fixed unit tests. Fixed namespaces.

* Refactored code, fixed uwp launcher. Added unit test

* Added sample launcher page and viewmodel.

* Fixed launcher Page typo

* Refactored launcher code. after testing exception behaviour I decided on rethrowing instead of swallowing the exception

* Refactored access modifiers. Also changed exception behaviour to enforce uri validity and exception behaviour across platforms.

* Updated docs, stripped even more code.

* Added validation. Adressed all other changes as required in pr

* Removed dead code

* Adressed last issues from pr

* Cleanup launcher

* cleanup iOS tests

* Early morning typos

* Name Alignment for BrowserLaunchMode (#408)

* Rename BrowserLaunchType to BrowserLaunchMode

* update nuget

* Rename parameter

* GH-287 OpenMaps Implementation (#361) (#404)

* GH-287 Maps Implementation (#361)

* Implemented maps as mentioned in branch name issue. also fixed a spelling mistake

* Added samples

* Added doc stubs

* Added comments for map types

* Refactored code

* Formatting of docs

* Added tests

* Changed coordinates to display something recognisable

* Added uri escaping, thus removing duplicate code. also had to turn off a warning due to inconsistent style cop and vs warning behaviour.

* Removed ref until in is added in c# 7.2

* Adressed issues in pr

* Updated launch code

* Adressed method to onliner

* Updated docs

* Removed ClearTop intent, added sample with adress

* Rename to align names. Added extensions and additional paramaters.

* Update tests

* Throw if options are null.

* Add overload for lat/long without options

* Enable multi-target for 25,26,27 (#411)

Alight nuget versions to xamarin.forms.

* No More Shared Projects (#410)

* Change to multi-targeted project for device tests :)

* Cleanup scripts

* Fix build error due to preprocessor directive (#417)

* Sync Sensor Speeds & Reset UWP to 0 (#419)

* Restore the default report interval to release resources while the sensor is not in use

* Cleanup sensor reading times.
Make them all the same and only in 1 place
Compass is still unique

* Create unique preference storage for each feature. (#434)

* Create unique preference storage for each feature.

* update storage per @Redth

* Update version
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants