feat(sensors_plus)!: add barometer support for all platforms #3079

aanas-sayed · 2024-07-08T20:19:17Z

Description

This PR implements barometer support for all platforms (Android, iOS, and web) within the sensors_plus plugin. The barometer sensor measures atmospheric pressure and can be used for various applications such as altimeters.

Note: There are limitations for web platform, as according to MZN docs, there is no such support for barometer support. This has been implemented similarly to the magnetometer which is also not implemented on any modern browsers, so it handles and throws the right errors if used.

Changes Made:

Android Implementation (Tested): Updated Android specific methods and handlers for barometer sensor in:
- ...sensors_plus\android\src\main\kotlin\dev\fluttercommunity\plus\sensors\SensorsPlugin.kt
- ...sensors_plus\android\src\main\kotlin\dev\fluttercommunity\plus\sensors\StreamHandlerImpl.kt
iOS Implementation (Not Tested): Updated packages\sensors_plus\sensors_plus\ios\sensors_plus.podspec to include barometer reference.
Added Android specific methods and handlers for barometer sensor in:
- ...sensors_plus\ios\Classes\FPPSensorsPlusPlugin.swift
- ...sensors_plus\ios\Classes\FPPStreamHandlerPlus.swift
Web Implementation (Tested): Added placeholder implementation and error handling for web platform. There is no plan for this to be implemented on the web from my understanding, but the implementation here will allow appropriate handling to be implemented by users.
Plugin Implementation (Tested): Added required implementations following the same pattern as used for other sensors for the barometer:
- BarometerEvent class in ...sensors_plus_platform_interface\lib\src\barometer_event.dart
- barometerEventStream methods in ...sensors_plus\lib\src\sensors_plus_web.dart, ...sensors_plus_platform_interface\lib\src\method_channel_sensors.dart, ...sensors_plus\lib\src\sensors.dart, ...sensors_plus\lib\src\web_sensors.dart, ...sensors_plus_platform_interface\lib\sensors_plus_platform_interface.dart and ...sensors_plus\lib\sensors_plus.dart
- Deprecated barometerEvents (not sure if this is needed, but there for consistency) in ...sensors_plus_platform_interface\lib\sensors_plus_platform_interface.dart and ...sensors_plus\lib\sensors_plus.dart
- ~~JS Interoperability for non-existing Barometer API in ...sensors_plus\lib\src\web_sensors_interop.dart~~
Tests (Tested): Unit tests have been written for the barometer in:
- ...sensors_plus\test\sensors_test.dart
- ...sensors_plus_platform_interface\test\sensors_plus_platform_interface_test.dart
Example App (Tested): Updated to include barometer readings in table in ...sensors_plus\example\lib\main.dart.
README.md: Updated to show barometer in example and reference it.

Related Issues

Implements required functionality for barometer (#955)

Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I titled the PR using Conventional Commits.
I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
All existing and new tests are passing.
The analyzer (flutter analyze) does not report any problems on my PR.

Note: flutter analyze reports errors for deprecated constants for network_info_plus. Did not touch or modify these as it is not related to sensor_plus.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
No, this is not a breaking change.

…m in sensors_plus_platform_interface.dart

…form

…/sensors_plus_platform_interface/test/

…sors_plus/example/

…/sensors_plus/test/

aanas-sayed · 2024-07-08T20:30:39Z

For quick reference, this is what the example app looks like now:

And for web platform you get similar messages as that for magnetometer which is also not implemented on modern browsers:

miquelbeltran · 2024-07-08T20:43:06Z

Oh wow, that's a lot! It will probably take a bit to review everything. But I think there should be no issues to incorporate these.

I did a quick read, and I'd say we shouldn't include the deprecated methods, even if the other sensors have them. Removing them will also reduce the size of the PR.

aanas-sayed · 2024-07-08T21:08:12Z

Started with thinking of just adding the android implementation and went down the rabbit hole haha after reading the contribution guide to try to get all 3.

Thanks for the quick response! Great if it can be added 😄 but of course not a rush for me.

Regarding the deprecated methods, they've been removed now.

aanas-sayed · 2024-07-10T11:02:26Z

Should I also update the root REAMDE.md?

From :

Flutter plugin for accessing accelerometer, gyroscope, and magnetometer sensors.

To :

Flutter plugin for accessing accelerometer, gyroscope, magnetometer and barometer sensors.

miquelbeltran · 2024-07-10T14:30:06Z

Should I also update the root REAMDE.md?

From :

Flutter plugin for accessing accelerometer, gyroscope, and magnetometer sensors.

To :

Flutter plugin for accessing accelerometer, gyroscope, magnetometer and barometer sensors.

Yep!

miquelbeltran

I would like to test the iOS implementation on my test iPhone eventually.

I left a comment on the web implementation, which I think should be removed since it does nothing.

I have also pushed a small README change.

packages/sensors_plus/sensors_plus/lib/src/web_sensors.dart

packages/sensors_plus/sensors_plus/lib/src/web_sensors_interop.dart

miquelbeltran · 2024-07-12T19:48:33Z

packages/sensors_plus/sensors_plus_platform_interface/lib/sensors_plus_platform_interface.dart

+
+  /// Returns a broadcast stream of events from the device barometer at the
+  /// given sampling frequency.
+  Stream<BarometerEvent> barometerEventStream({
+    Duration samplingPeriod = SensorInterval.normalInterval,
+  }) {
+    throw UnimplementedError('barometerEvents has not been implemented.');
+  }


Since we modify the platform interface, this change has to be marked as a breaking change.

My understanding was this will not affecting current users' implementations using the library and so not a breaking change. I'm happy to mark it as such if this is how its maintained, but out of curiosity, could you please elaborate why?

The sensor_plus and sensor_plus_platform_interface are in two different packages. If the signature of the method changes, but the client project doesn't pull the correct version, this leads to a compilation error.

This means that we either need to pin the exact version or do a major release.

We had this issue before with other packages.

FYI @vbuberen is this assumption correct? Does Melos support pinning and updating the exact version of *_platform_interface ?

I am not really sure here, but during our usual release routine I will check it myself that the constraints for *_platform_interface are updated.

Thanks both. I've flagged it as a breaking change as @miquelbeltran suggested. Is there anything further you need me to do in regards to this?

No, nothing else needed. Thanks

vbuberen

Looks mostly good to me. Thanks a lot for this contribution.

packages/sensors_plus/sensors_plus/ios/sensors_plus.podspec

packages/sensors_plus/sensors_plus/ios/Classes/FPPStreamHandlerPlus.swift

…nsors_plus.podspec Co-authored-by: Volodymyr Buberenko <[email protected]>

vbuberen · 2024-07-15T08:53:08Z

From my side everything is good.
@miquelbeltran Can we merge this?

miquelbeltran · 2024-07-15T10:22:56Z

I want to test on iOS still. @aanas-sayed mentioned that the iOS implementation is not tested either.

miquelbeltran · 2024-07-15T12:59:45Z

I want to test on iOS still. aanas-sayed mentioned that the iOS implementation is not tested either.

Well scratch that, I realized I no longer have an iOS dev account, so I cannot test this.

vbuberen · 2024-07-15T14:53:34Z

Well scratch that, I realized I no longer have an iOS dev account, so I cannot test this.

I will test the PR on the iPhone.

vbuberen · 2024-07-16T15:36:49Z

Did some testing on iOS and indeed there is a need to do some additional work.

There is a requirement to include NSMotionUsageDescription into Info.plist of the app (in this case example app) to explain why access to motion data is required. We need to add both this entry into Info.plist and mention this requirement in the documentation.
On example app start there is a request to access motion data (see the screenshot attached), but even after providing such permission I still see a message saying that no barometer detected, so there is definitely a missing piece in implementation.

aanas-sayed · 2024-07-16T18:27:22Z

Thanks for testing it. I'll look into these and try to fix them.

vbuberen · 2024-07-16T18:33:29Z

Thanks for testing it. I'll look into these and try to fix them.

In case it is hard to do due to not having a real iOS device I could try and fix everything myself. But will be able to do it not earlier than on weekend.

aanas-sayed · 2024-07-16T18:49:23Z

I don't have an apple dev account but got Mac (fairly new to it) and iOS devices. Never tried to test an iOS app tbh. Heard you can get it going without an apple dev account so going to try that out. If it doesn't work, I might need your support to fix the issues. I'll let you know how I get along. Thanks!

…s.swift

aanas-sayed · 2024-07-17T18:27:25Z

Fixed both the issues now I believe:

The missing entry in Info.plist has been added.
The missing barometer sensor issue was not a missing barometer issue it seems. It was an issue with incorrect type casting when sending the data which was being caught in the dart code which handled all errors as missing barometer errors. Should be fixed now.

vbuberen · 2024-07-18T08:05:47Z

I will be able to retest everything on a real device in ~6-8 hours, so will get back with feedback soon.

vbuberen · 2024-07-19T06:57:58Z

I have done testing on both Android and iOS devices. Everything seems to work fine. There are a few improvement points, which can be done in a follow-up PR:

Update README to mention that now on iOS devs need to provide NSMotionUsageDescription string in their Info.plist (I will make sure it is highlighted in changelogs as well when we release this feature).
Add units to values in the example app and show that it is hPa numbers.

@miquelbeltran Do you have something to add or I can merge this one?

miquelbeltran · 2024-07-19T07:28:05Z

Nope, we can go ahead!

aanas-sayed added 8 commits July 8, 2024 19:57

feat(sensors_plus): add barometer support for Android platform

234321e

fix(sensors_plus): correct deprecated message for barometerEventStrea…

72fa27d

…m in sensors_plus_platform_interface.dart

feat(sensors_plus): implement non-existing barometer API for web plat…

fdd5bb5

…form

test(sensors_plus): add barometer unit test in /packages/sensors_plus…

ecfe100

…/sensors_plus_platform_interface/test/

feat(sensors_plus): add barometer support for iOS platform (untested)

252c2e7

feat(sensors_plus): add barometer usage to /packages/sensors_plus/sen…

f193d31

…sors_plus/example/

docs(sensors_plus): update README.md

dca0df8

test(sensors_plus): add barometer unit test in /packages/sensors_plus…

3eb3d5d

…/sensors_plus/test/

chore(sensors_plus): remove deprecated barometer methods

97f309c

miquelbeltran requested review from miquelbeltran and vbuberen July 9, 2024 06:06

docs(sensors_plus): update plus plugins README.md

fc5e91e

miquelbeltran self-assigned this Jul 12, 2024

miquelbeltran added 2 commits July 12, 2024 21:40

Merge branch 'main' into dev/barometer-955

1e319aa

add barometer comment for web

8acef9e

miquelbeltran reviewed Jul 12, 2024

View reviewed changes

miquelbeltran added the PR: needs manual testing label Jul 12, 2024

fix(sensors_plus): remove unecessary barometer code for web

f2cf878

aanas-sayed changed the title ~~feat(sensors_plus): add barometer support for all platforms~~ feat(sensors_plus)!: add barometer support for all platforms Jul 14, 2024

vbuberen reviewed Jul 14, 2024

View reviewed changes

packages/sensors_plus/sensors_plus/ios/sensors_plus.podspec Outdated Show resolved Hide resolved

vbuberen reviewed Jul 14, 2024

View reviewed changes

packages/sensors_plus/sensors_plus/ios/Classes/FPPStreamHandlerPlus.swift Outdated Show resolved Hide resolved

aanas-sayed and others added 2 commits July 14, 2024 15:53

chore(sensors_plus): update packages/sensors_plus/sensors_plus/ios/se…

df72c41

…nsors_plus.podspec Co-authored-by: Volodymyr Buberenko <[email protected]>

fix(sensors_plus): remove unnecessary CoreLocation for ios

cd65756

vbuberen approved these changes Jul 15, 2024

View reviewed changes

Merge branch 'fluttercommunity:main' into dev/barometer-955

b1769e0

aanas-sayed added 2 commits July 17, 2024 18:27

fix(sensors_plus): missing NSMotionUsageDescription for example

1147535

fix(sensors_plus): incorrect type for pressure in FPPStreamHandlerPlu…

f1dc2c9

…s.swift

vbuberen merged commit 5fa797d into fluttercommunity:main Jul 19, 2024
17 checks passed

aanas-sayed deleted the dev/barometer-955 branch July 19, 2024 07:56

aanas-sayed mentioned this pull request Jul 19, 2024

chore(sensors_plus): update README and example app for units and platform specific recomendations #3096

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sensors_plus)!: add barometer support for all platforms #3079

feat(sensors_plus)!: add barometer support for all platforms #3079

aanas-sayed commented Jul 8, 2024 •

edited

Loading

aanas-sayed commented Jul 8, 2024 •

edited

Loading

miquelbeltran commented Jul 8, 2024

aanas-sayed commented Jul 8, 2024

aanas-sayed commented Jul 10, 2024

miquelbeltran commented Jul 10, 2024

miquelbeltran left a comment

miquelbeltran Jul 12, 2024

aanas-sayed Jul 13, 2024

miquelbeltran Jul 14, 2024

miquelbeltran Jul 14, 2024

vbuberen Jul 14, 2024

aanas-sayed Jul 14, 2024

vbuberen Jul 15, 2024

vbuberen left a comment

vbuberen commented Jul 15, 2024

miquelbeltran commented Jul 15, 2024

miquelbeltran commented Jul 15, 2024

vbuberen commented Jul 15, 2024

vbuberen commented Jul 16, 2024

aanas-sayed commented Jul 16, 2024

vbuberen commented Jul 16, 2024

aanas-sayed commented Jul 16, 2024

aanas-sayed commented Jul 17, 2024

vbuberen commented Jul 18, 2024 •

edited

Loading

vbuberen commented Jul 19, 2024 •

edited

Loading

miquelbeltran commented Jul 19, 2024

feat(sensors_plus)!: add barometer support for all platforms #3079

feat(sensors_plus)!: add barometer support for all platforms #3079

Conversation

aanas-sayed commented Jul 8, 2024 • edited Loading

Description

Changes Made:

Related Issues

Checklist

Breaking Change

aanas-sayed commented Jul 8, 2024 • edited Loading

miquelbeltran commented Jul 8, 2024

aanas-sayed commented Jul 8, 2024

aanas-sayed commented Jul 10, 2024

miquelbeltran commented Jul 10, 2024

miquelbeltran left a comment

Choose a reason for hiding this comment

miquelbeltran Jul 12, 2024

Choose a reason for hiding this comment

aanas-sayed Jul 13, 2024

Choose a reason for hiding this comment

miquelbeltran Jul 14, 2024

Choose a reason for hiding this comment

miquelbeltran Jul 14, 2024

Choose a reason for hiding this comment

vbuberen Jul 14, 2024

Choose a reason for hiding this comment

aanas-sayed Jul 14, 2024

Choose a reason for hiding this comment

vbuberen Jul 15, 2024

Choose a reason for hiding this comment

vbuberen left a comment

Choose a reason for hiding this comment

vbuberen commented Jul 15, 2024

miquelbeltran commented Jul 15, 2024

miquelbeltran commented Jul 15, 2024

vbuberen commented Jul 15, 2024

vbuberen commented Jul 16, 2024

aanas-sayed commented Jul 16, 2024

vbuberen commented Jul 16, 2024

aanas-sayed commented Jul 16, 2024

aanas-sayed commented Jul 17, 2024

vbuberen commented Jul 18, 2024 • edited Loading

vbuberen commented Jul 19, 2024 • edited Loading

miquelbeltran commented Jul 19, 2024

aanas-sayed commented Jul 8, 2024 •

edited

Loading

aanas-sayed commented Jul 8, 2024 •

edited

Loading

vbuberen commented Jul 18, 2024 •

edited

Loading

vbuberen commented Jul 19, 2024 •

edited

Loading