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

☰♭ Adding a Bluetooth Scanner to dev options #1128

Merged
merged 38 commits into from
Mar 22, 2024

Conversation

the-bay-kay
Copy link
Contributor

Purpose

In our upcoming project with GSA GPG, we're hoping to utilize either (a) Classic Bluetooth , or (b) Bluetooth Low-Energy (BLE) Beacons to track vehicle data. Because Classic Bluetooth is found in most modern vehicles, we are hoping to utilize it in our vehicle tracking.

To test the validity of this, we're implementing a rudimentary scanner within the Developer Zone of the OpenPATH app. This will be used by the team to record data on the behavior of Classic Bluetooth devices found within personal cars. Then, with this data, we will be able to make a more informed decision on how to proceed with our project!

Implementation

Myself and @louisg1337 have come up with a brief set of"Project Requirements" for this project:

  • A button in the Dev Zone, that opens a Bluetooth Scanner page
    • This button should only be visible when the Survey Config is set for projects that require bluetooth; GSA GPG, etc.
  • The Bluetooth Scanner Page will need:
    • A function that interfaces with whichever Bluetooth API we choose. This should fetch all devices broadcasting within range of the phone
    • A React Component (may be as simple as a <Text/>), that displays a given Bluetooth Device's information
    • A Scrolling list of the "Device Components"
      • This list will ideally automatically update as devices come in and out of range (I.e., we'd call the scan method frequently). However, due to time constraints , it may be simpler to add a manual refresh button.
    • I believe we may also need prompt the user, requesting permission to use Bluetooth. This may be OS Dependent, will update as we learn more.

Our current "bottleneck" is the lack of a comprehensive Cordova Bluetooth plugin. The ability to work with Core Bluetooth was added to iOS was added somewhat (within the past 5 years, according to this article on Apple's site). We've found a few options for Android Scanners, detailed in the issue below. If we are unable to find a "one-size fits all" plugin, we're planning to utilize two separate ones, depending on OS; we've already found some decent options for Android.

For more information on this project please see Issue #1046 .

See Issue [e-mission#1046](e-mission/e-mission-docs#1046 (comment))
for more details.  This commit adds the Dev-Zone settings option, which
will eventually link to the Bluetooth Scanner page.

TODO:
- Add i18next translation for control.bluetooth
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.49%. Comparing base (ab3c891) to head (295b96f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1128   +/-   ##
=======================================
  Coverage   77.49%   77.49%           
=======================================
  Files          29       29           
  Lines        1693     1693           
  Branches      370      370           
=======================================
  Hits         1312     1312           
  Misses        381      381           
Flag Coverage Δ
unit 77.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@the-bay-kay the-bay-kay marked this pull request as draft February 9, 2024 20:48
The button added in settings now will open a Modal "Page" that
will (eventually) contain the bluetooth devices within scanning
distance!
@the-bay-kay
Copy link
Contributor Author

UI Progress Update

Just added the bare-bones settings page! I chose to format this page similarly to the EnketoModal.tsx module (thanks to Jack for writing that component!): since we have this "pop-up" page in settings already, I think it makes sense to design this in a similar fashion. I've still got plenty of work to do, but I wanted a bare-bones implementation so Louis could use it as "scratch paper" for the scan() testing!

Screen.Recording.2024-02-09.at.3.21.30.PM.mov

TODO

  • Fix title of "bluetooth" setting row, add it to the translations
  • Fix the UI of the settings page; including the color of the button, background of the page, etc.
  • Create a component to nest the bluetooth data
  • Figure out how to populate a list of these bluetooth components
  • Time permitting, I think we could go back and refractor the "bluetooth" component of the

the-bay-kay and others added 4 commits February 9, 2024 15:35
It seems I accidentally turned off my local VSCode Prettier... Fixing
that now
…eated bluetoothScanner.ts which houses the plugin scan request. I then added in functionality to the BluetoothScanPage to output the data that we get from the scan function.
@louisg1337
Copy link
Contributor

Scanner Update

We now have a roughly working bluetooth scanner! Big thanks to @the-bay-kay for getting all the react side of stuff setup for us! If you look at the picture it picks up my JBL headphones which I think are bluetooth classic, so it seems like a success. To use the scanner, navigate to the bluetooth page, click on permissions, click Allow, then click scan and wait for the results.

bluetooth

Quick run down of what I've done so far:

  • Added the plugin cordova-plugin-bluetooth-serial to the app's dependencies.
  • Implemented gatherData() in bluetoothScanner.ts which calls the plugin and initiates a bluetooth scan.
  • Put together a quick way in BluetoothScanPage.tsx to display all the data received from gatherData() just to see if it actually works on a test phone.
  • Added a bluetooth scanning permissions request in my e-mission-datacollection fork as we need to request permissions to scan (link to the branch).

Notes
A big issue me and Katie were wrestling with on Friday was how to edit the UI and Native Code at the same time. What worked for me is from a fresh install, do the normal Android native build procedure, and whenever you want to change the UI run npm run build-dev-android and then npx cordova emulate android. I never realized that the npm command recompiles both native and UI. I tried installing the plugin first and then setting up the UI and running the server but it just didn't seem to like me. I'm sure I was doing something wrong, but the following steps above worked good enough here.

Since I had to mess around with e-mission-datacollection to be able to request permissions from the user, we now need to use my fork of the plugin to build to native. To add my fork of the plugin to the project, here are the steps:

  1. Clone my repo.
  2. Fresh install Android native code.
  3. Run npx cordova plugin remove cordova-plugin-em-datacollection
  4. Run npx cordova plugin add [path to cloned repo]

After those steps I think we should be all set! I think we have to do it this way until we can get a new release pushed out to e-mission-datacollection, but once my forked version of the plugin is added it doesn't go away unless you do a fresh install.

Todos

  • Tidy up bluetoothScanner.ts and utilize more typescript conventions in it.
  • Fix up the permissions because right now there is no error handling, so if the user denies the program breaks.
  • Test out on the side of the street to see if it picks up cars.

@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Feb 12, 2024

First things first, great work getting the scanning working!

If you look at the picture it picks up my JBL headphones which I think are bluetooth classic, so it seems like a success.

Correct! Bluetooth Classic is used for the vast majority of audio data transfer . JBL doesn't disclose what protocol they use, but I'd put good money on it being classic -- which means we should be in business!

Todos:

  • Tidy up bluetoothScanner.ts and utilize more typescript conventions in it.
  • Fix up the permissions because right now there is no error handling, so if the user denies the program breaks.
  • Test out on the side of the street to see if it picks up cars.

I'll get started on these! Just brianstorming some other things left to-do, I think we need to:

  • Clean up the UX / UI, to have a cohesive style
  • Add i18next translation support
  • Expand on thebluetoothScanner.ts typescript
  • Test the scanner on the street and in my car!
  • Confirm we can build to both iOS and Android
    • We may need to update iOS permissions
  • Fix Bluetooth Permissions

With time permitting, we could:

  • Add Jest tests & mocks for the bluetooth function
  • Refactor EnketoModal and BluetoothPage into one "Pop-up Page" component, that could be populated with content (A survey, or scanner, etc.)... not necessary, but may be a good exercise!
  • Change Bluetooth icon based on device signal strength Not possible with current plugin
  • If we'd like, we can hide the scanner on non-GSA GPG OpCodes

I'm going to start working down the list, focusing on the UX / UI for now -- I'll keep this thread updated as I go!

@the-bay-kay
Copy link
Contributor Author

Did some work refactoring and cleaning up the UI! I'm going to do a test build to iOS before committing and pushing these changes -- as a quick update, I've included a screenshot of the new UI with some dummy values.

With time permitting, I would like to dynamically change the color of the Bluetooth icon on the left hand side, so that it matches the signal strength (Red - low, Green - high). This would require refactoring the scan() function to return the whole "device object", rather than a log-style output. It's functional as is, but this change could help us get more data on the devices, if we'd like.

I'll go ahead and edit my previous comment on this thread, crossing off / editing the "TODO"s!

image

- Formatted buttons
- Added `BluetoothCard.tsx`
- Centered buttons & Device Info
- Fixed "Dismiss" Button, updated to arrow
Added english translations for scanner components
@the-bay-kay
Copy link
Contributor Author

Added i18next support and English translations! Below is a video of the current UI, with some dummy examples. As usual, I'll update the to-do list.

Some asides:

  • As Louis and I have discussed, the scanner currently is only supported on Android: this is because we couldn't find a cross-paltform Bluetooth Classic Scanner. If we want to edit the code to scan only for BLE, I believe this plugin we're using should suffice.
  • I made mention of changing the bluetooth symbol's color, based on signal strength. From what Louis and I can tell, the function we're using to scan (docs here) doesn't return the signal strength. We may be able to implement this with a different plugin, but I'll put this change on the backburner for now.
    • While I'm thinking about it -- our current implementation only uses the plugin's discoverUnpaired() function... After I build to hardware, I'll do some tests and see if this is an issue.
  • We may also want to tweak the UI further, or at least adjust the card size -- with the iPhone emulator, it's not 100% obvious that you can scroll and view more devices (i.e., don't see a card peaking out on the bottom). Since this is a dev feature, I'm not convinced this is a dealbreaker -- I'll focus on testing first, and adjust this as time permits!
Screen.Recording.2024-02-12.at.4.52.49.PM.mov

@the-bay-kay
Copy link
Contributor Author

Working on making the bluetooth scan a "toggle", and ran into a fascinating bug.

When attempting to use the useEffect() hookin the Profile Settings, it seems the hook runs as soon as we open the developer zone. Interesting. Adding a video to this post -- will update once I understand the issue better! FWIW, this doesn't seem to be a problem.., I'm running into a separate issue where the toggle turn off the scan, but this shows the toggle working correctly!

Screen.Recording.2024-02-13.at.11.34.23.AM.mov

louisg1337 and others added 2 commits February 19, 2024 15:34
- Updated plugin to a fork of the original: ideally, this fork
should support Bluetooth Classic for both iOS & Android
@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Feb 20, 2024

PR Updates

Louis and I have been discussing & testing this PR, and wanted to share some notes / updates!

  • We've successfully built to iOS emulator with the Bluetooth Serial Classic Fork (link) of our current plugin. Because this plugin is a successor to the original, we can assume our Android code will behave similarly. We're still focusing on getting Android running first, as there are additional difficulties with iOS.
    • The goal is to have scanning work in both OSs, but (as mentioned in issue 1046), there are some additional steps that may need to be taken for iOS. If only Android is functional, the team will still be able to test the reliability of Car Stereos as beacons -- a crucial first step if we are to move forward with Bluetooth Classic.
  • There have been issues with the "continuous scanning". Certain devices haven't had issues, but others (LGv30, Nexus 6) have been incredibly unreliable. As such, we're planning to revert back to a "scan-once" button, in the interest of wrapping this project up
    • Our current theory as to the cause of this lies within the Bluetooth plugin we're using. The documentation mentions that scanning for unpaired devices may take a significant amount of time:

      The discovery process takes a while to happen. You can register notify callback with setDeviceDiscoveredListener.

      We may be able to fix the "toggle scan" by increasing the callback interval, but as mentioned above, we're reverting to the button in the interest of time. This behavior may also change as we switch to the BluetoothSerialClassic fork of the plugin, but presumably it will function similarly.

Permissions Bugs

Below are a few subtle bugs I've noticed, as I've been working on this. Just dumping these here for now, so they don't get lost as we fix the bigger things!

  • On at least android, I've had an issue with the permissions screen. When making the battery permissions, it gives an error prompting to check the "nearby devices" permission, and doesn't update for a short period of time. Since we've added a permission, this isn't terribly surprising to me: we should go back and investigate how to include Bluetooth
  • Similarly, we need to adjust how we check for Bluetooth permissions on iOS: the current call to Cordova in BluetoothSettingsRow doesn't work (and throws an error when triggered on iOS), so we'll naturally need to update that as we add iOS functionality.

Going Forward

As of this moment, our next steps are:

  • Confirm the app builds with the "classic" fork of the Bluetooth plugin
    • Likewise, confirm that the already written Android features work with this updated plugin
  • Revert the "toggle scan" to a "scan-once" button
  • Fix a "text-wrapping" issue with Bluetooth Cards on Android
  • Begin writing the iOS scanner

- Cards were not splitting logs correctly, fixed to split via index
  - We should be fine with this rudimentary string parsing technique;
  from my testing, IDs have not gone past the length given.
Merge to keep branch up to date with master
@the-bay-kay
Copy link
Contributor Author

Still working on the WSOD issue - when looking for more information on the In App Browser (IAB.close()), I haven't found any fruitful leads. Looking for calls to IAB within the plugins also gives little insight. Below are the places where this function is called, FWIW. I'll keep on hunting for the resolution!

(base) $ grep -rnw ./ -e 'IAB.close()'

.//platforms/ios/emission/Plugins/cordova-plugin-inappbrowser/CDVWKInAppBrowser.m:75:        NSLog(@"IAB.close() called but it was already closed.");
.//plugins/cordova-plugin-inappbrowser/RELEASENOTES.md:469:* [CB-5733](https://issues.apache.org/jira/browse/CB-5733) Fix IAB.close() not working if called before show() animation is done
.//plugins/cordova-plugin-inappbrowser/RELEASENOTES.md:740:* [CB-5733](https://issues.apache.org/jira/browse/CB-5733) Fix IAB.close() not working if called before show() animation is done
.//plugins/cordova-plugin-inappbrowser/src/ios/CDVWKInAppBrowser.m:75:        NSLog(@"IAB.close() called but it was already closed.");
.//plugins/cordova-plugin-inappbrowser/src/browser/InAppBrowserProxy.js:103:                        IAB.close();
.//plugins/cordova-plugin-inappbrowser/src/browser/InAppBrowserProxy.js:177:                        IAB.close();
.//node_modules/cordova-plugin-inappbrowser/RELEASENOTES.md:469:* [CB-5733](https://issues.apache.org/jira/browse/CB-5733) Fix IAB.close() not working if called before show() animation is done
.//node_modules/cordova-plugin-inappbrowser/RELEASENOTES.md:740:* [CB-5733](https://issues.apache.org/jira/browse/CB-5733) Fix IAB.close() not working if called before show() animation is done
.//node_modules/cordova-plugin-inappbrowser/src/ios/CDVWKInAppBrowser.m:75:        NSLog(@"IAB.close() called but it was already closed.");
.//node_modules/cordova-plugin-inappbrowser/src/browser/InAppBrowserProxy.js:103:                        IAB.close();
.//node_modules/cordova-plugin-inappbrowser/src/browser/InAppBrowserProxy.js:177:                        IAB.close();

- Updated calls to Bluetooth Plugin, to ensure paired
devices are included in the scan list
@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Feb 29, 2024

Some more notes / updates on the WSOD issue:

  • The error only occurs on iOS builds
  • The issue occurs across branches (master, bluetooth_scanner, and older branches I have built on before scan_opcode_fix)
  • This error occurs across built platforms: So far, I've tested
    • iPhone 15 emulator running iOS 17
    • iPhone 8, running iOS 15.5
    • iPhone SE running iOS 16.1.1
  • The issue occurs across repositories; I've done fresh clones of both e-mission-phone and my personal fork, with no discernible difference.
  • @louisg1337 has successfully built the master branch to iPhone hardware, without the WSOD occurring

I just performed a fresh install (Deleted XCode, cleared the simulators, cache, etc.), with no change. At first, I was certain this would fix the issue; since others can build on their devices, I figured it was an issue with my XCode install.. I'll be doing some more research to find a solution!

@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Feb 29, 2024

Test Updates

Just wanted to write a few words about these last two commits! I initially had some concerns that our API call (.discoverUnpairedDevices()) wasn't looking at our paired device lists, and as such, was missing all of our relevant data. As such, we added the past two commits to include a call to .list(), which should return the list of paired devices as well. (For more info, please see the plugin docs here)

Unfortunately, it seems that the "visibility" of a signal still applies to pair devices; after running some tests with my car stereo, a "non-visible" device still doesn't appear during a scan, even when it's paired. Still glad to have crossed this off the list of tests!

@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Mar 1, 2024

Finally managed to re-build! Turns out, this build issue and issue 1053 were closely related. The issue was with my MacOS version, and the plugin macos-release. The main issue was with the versioning: before running bash setup/setup_serve.sh, I needed to add the following to setup/autoreload/macos-index.js:

'use strict';
const os = require('os');

const nameMap = new Map([
	[23, ['Sonoma', '14.3.1']], // <- Version added
	[22, ['Ventura', '13']],
	[21, ['Monterey', '12']],
	// More versions below
]); // more code below

I also manually checked (and edited) node_modules/macos_release/index.js, to add the "Sonoma" MacOS version. With this, I also found that I needed to run and test the serve, before running and executing the native setup. I'm still unsure why this order matters, I'll experiment more and report back. After updating this plugin, and re-adjusting settings on XCode (Min Build version, Bluetooth permission strings, removing push notification certificates), I was finally able to build to hardware!

I'll go ahead and add these notes to the issue mentioned above, and close it; now that this issue is in the rear view mirror, I'll go ahead and start fixing / testing the iOS build!

@louisg1337
Copy link
Contributor

Along with these changes, there has been some work done to incorporate bluetooth permissions into e-mission-datacollection, and here is the respective pull request for that.

@the-bay-kay the-bay-kay marked this pull request as ready for review March 6, 2024 16:53
@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Mar 6, 2024

Louis and I chatted earlier this morning, and we agreed the last change necessary for this PR is that we may need to handle iOS's Bluetooth permission acceptance / rejection better. As written, we don't have much of a structure in place for this: if someone rejects the Bluetooth permission, we're unsure if that state is recoverable. I went to test on my hardware, and ran into the same WSOD as before... I'll run some more tests, and then update this PR as necessary!

@the-bay-kay
Copy link
Contributor Author

Louis and I have both given this a final once-over, and the PR is ready for review! That being said, this does rely on the em-data-collection PR that Louis published (link) -- that should be merged before this one!

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

Couple of high level changes:

  • avoid race conditions
  • use the device settings correctly

Once you address those and bump up the version number, I can merge this

package.cordovabuild.json Show resolved Hide resolved
www/js/bluetooth/blueoothScanner.ts Outdated Show resolved Hide resolved
www/js/control/BluetoothScanSettingRow.tsx Outdated Show resolved Hide resolved
www/js/control/ControlSyncHelper.tsx Outdated Show resolved Hide resolved
- Moved permissions check to BLE Screen, fixed error
with permission appearing upon install
- Updated OS checks, fixed config export
@shankari
Copy link
Contributor

@the-bay-kay please remember to move the PR back into ready for review after you are done addressing review comments

www/js/bluetooth/blueoothScanner.ts Outdated Show resolved Hide resolved
www/js/bluetooth/blueoothScanner.ts Outdated Show resolved Hide resolved
www/js/bluetooth/blueoothScanner.ts Outdated Show resolved Hide resolved
@the-bay-kay
Copy link
Contributor Author

the-bay-kay commented Mar 20, 2024

Attached below is the functioning, current version of the Bluetooth Classic scanner!

Interestingly, I've noticed that window['bluetoothClassicSerial'].list does not behave exactly like how we expected. In the screenshot, MVH-210EX is shown as paired (see: the icon). In reality, this phone is not connected; I have paired with this device in the past, but am not actively connected. This is a nuance I didn't consider.

Currently re-reading the docs to see if another function would suffice (notabely, .isConnected() is not what we want, it only confirms that something is connected).

Screenshot_20240320-100702

- Changed logs to only pass devices, now parsed in BluetoothCard
- Updated promise handling in blueoothScanner
- Reverted iOS permissions check
@the-bay-kay
Copy link
Contributor Author

As written, this is ready for merge -- it's functional, if not performing exactly how we'd like. At a glance, I haven't found anything within the function that easily displays connected devices. I'll spend a few more minutes reading through the docs, but then will pivot to try and finish the BLE Scanner.

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

LGTM! I only see one minor comment/fix below, which you can handle in the next PR

});

Promise.all([unpairedDevicesPromise, pairedDevicesPromise])
.then((logs: Array<any>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, future fix: can't this be:

Suggested change
.then((logs: Array<any>) => {
.then((logs: Array<BluetoothClassicDevice>) => {

@shankari shankari merged commit 78b5da1 into e-mission:master Mar 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tasks completed
3 participants