Skip to content

feat(sensors_plus)!: Migrate to dart:js_interop #2697

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

Merged

Conversation

koji-1009
Copy link
Contributor

Description

APIs such as Accelerometer are defined independently using dart:js_interop.


Do you know how to see the sensor change in the browser, I tried emulation with chrome dev tools but could not see the value change.

https://developer.chrome.com/docs/devtools/sensors

Related Issues

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.

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.

@koji-1009 koji-1009 force-pushed the feat/sensors_plus_js_interop branch from c3a3acb to 98d768c Compare March 14, 2024 12:38
@koji-1009 koji-1009 force-pushed the feat/sensors_plus_js_interop branch from 98d768c to 3ff6c97 Compare March 14, 2024 12:40
@miquelbeltran
Copy link
Member

Oh, this looks nice! Thanks!

I'll have a look regarding the sensors if there is a way to test this.

Also, this doesn't depend on web at all?

@miquelbeltran miquelbeltran self-assigned this Mar 14, 2024
@koji-1009
Copy link
Contributor Author

Thanks for the quick response!
I checked web and the Sensor API type does not exist. For this reason, I am not using web 👍

@miquelbeltran
Copy link
Member

I did a quick run on my Linux machine + chrome both main and your PR, and in both cases I cannot make it work.

On main I see the following error:

[log] Magnetometer() is not supported by the User Agent.
[log] The accelerometer API is supported but something is wrong!
[log] The linear acceleration API is supported but something is wrong!
[log] The gyroscope API is supported but something is wrong!

on your PR I get the following:

[log] Magnetometer() is not supported by the User Agent.
[log] TypeError: constr is not a constructor
[log] The accelerometer API is supported but something is wrong!
[log] Could not connect to a sensor
[log] The linear acceleration API is supported but something is wrong!
[log] Could not connect to a sensor
[log] The gyroscope API is supported but something is wrong!
[log] Could not connect to a sensor

I will check on MacOS + Chrome

@miquelbeltran
Copy link
Member

Similar errors, slightly different, on macOS + Chrome:

main

[log] Magnetometer() is not supported by the User Agent.
[log] The linear acceleration API is supported but something is wrong!
[log] The gyroscope API is supported but something is wrong!

The PR code:

[log] Magnetometer() is not supported by the User Agent.
[log] TypeError: constr is not a constructor
[log] The linear acceleration API is supported but something is wrong!
[log] Could not connect to a sensor
[log] The gyroscope API is supported but something is wrong!
[log] Could not connect to a sensor

In theory, it is all well-supported in chrome, but I have the sensation it never worked before.

Looking at the documentation https://developer.mozilla.org/en-US/docs/Web/API/Sensor_APIs we are supposed to query for permissions, but that doesn't seem to happen here at all.

image

"could not connect to a sensor" is the only error visible.

@koji-1009
Copy link
Contributor Author

Thank you!
I will try to get the permissions. If it works, I'll add a new commit.

@koji-1009
Copy link
Contributor Author

hmm, I tested following codes. The permissions are already granted.

import 'package:web/web.dart';

~~~

@override
Stream<AccelerometerEvent> accelerometerEventStream({
  Duration samplingPeriod = SensorInterval.normalInterval,
}) {
  if (_accelerometerStreamController == null) {
    _accelerometerStreamController = StreamController<AccelerometerEvent>();
    _featureDetected(
      () async {
        final status = await window.navigator.permissions
            .query(
              PermissionDescriptor(
                name: 'accelerometer',
              ),
            )
            .toDart;
        print('accelerometer: name: ${status.name}, state: ${status.state}');

result.

accelerometer: name: sensors, state: granted

I will continue to investigate.
The js_interop is interesting.

@koji-1009
Copy link
Contributor Author

https://koji-1009.github.io/plus_plugins/
koji-1009#1

I uploaded example app. When I open it in chrome on my Android phone, it seems to be working as expected.

@koji-1009 koji-1009 marked this pull request as ready for review March 14, 2024 15:45
@miquelbeltran
Copy link
Member

Great job! Checked on my phone and it works as well. Is the code in the PR up-to-date?

@koji-1009
Copy link
Contributor Author

koji-1009 commented Mar 14, 2024

Yes, this PR code is up to date.
When deploying the demo, /sensors_plus/example/web/index.html needs to be newer, so it is updated. However, there are no additional changes in /sensors/lib.

https://github.com/koji-1009/plus_plugins/pull/1/files#diff-c8f651872a9b98a8388ebdbbe12c950e1ae16e5d62c746cbd71192524b9b058a

@miquelbeltran miquelbeltran merged commit 48edfa2 into fluttercommunity:main Mar 15, 2024
@koji-1009 koji-1009 deleted the feat/sensors_plus_js_interop branch March 15, 2024 13:26
@koji-1009
Copy link
Contributor Author

Thanks!

@miquelbeltran
Copy link
Member

Thanks to you!

@guibertjulien
Copy link

Hello, I have this log with sensors_plus 6.0.0 with Chrome, is it normal ? I try to test sensor with a PWA application.
Capture d’écran 2024-08-05 à 08 24 32

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.

[Request]: (sensors_plus) Migrate to package:web to support WASM
3 participants