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

🔵🦷 UI changes for BLE integration #1145

Merged
merged 22 commits into from
Apr 15, 2024

Conversation

JGreenlee
Copy link
Collaborator

No description provided.

shankari and others added 11 commits April 1, 2024 15:52
- Move the major and minor boxes to the same row to take up less space
- Split the fake callbacks into enter, exit and range
- generalize `fakeMonitorCallback` to work for both enter and exit
- do not call rangeCallback directly from monitor callback since there
  are now separate buttons
- for the range callback, use the device major and minor values if they
  exist
- for the range callback, reset monitor and range results to avoid
  duplicates

This is the first step for testing the data models defined in:
e-mission/e-mission-docs#1062 (comment)
To be consistent with processed trips, and so we can use them in the same way, unprocessed trips should have the 'sections' property.

Unprocessed trips / 'draft' trips are assumed to be unimodal, so we can fairly easily reconstruct a section spanning the entirety of the trip and include that as the only section.

In doing so, I modified points2TripProps. I realized that many of the props will actually be the same between the trip and the section. So I did a bit of refactoring; to construct the unprocessed trip I first construct baseProps (which are used in both the section and the trip); then I construct the section; then finally I construct the trip in the return statement.

Something else I noticed is that all the trip props are computed in points2TripProps except the start_loc and end_loc which were computed in the outer function transitionTrip2TripObj. Since we already have variables for the start and end coordinates in the points2TripProps function, it seems more logical to handle start_loc and end_loc there with the rest of the trip props.

Then we can declare the return type of points2TripProps as UnprocessedTrip; it has all the required properties now.
The main thing I was doing in this commit was adding 'sections' to the type signature of `UnprocessedTrip`. But I also noticed some other things amiss.

`UnprocessedTrip` was missing some other properties; end_ts and start_fmt_time

"LocationCoord" is not needed as it's the same as `Point` from 'geojson'.

`SectionData` was missing a bunch of properties. Once those are filled in, the 'sections' property in `CompositeTrip` and `UnprocessedTrip` can be typed as `SectionData[]`
After a bunch of refactoring, I think these functions could use a naming update.
I always found this part of the codebase fairly opaque anyway and I think it can now be made more easily comprehensible.

'points2TripProps' now returns a full UnprocessedTrip object so it is renamed 'points2UnprocessedTrip'

And 'transitions2Trips' is renamed 'transitions2TripTransitions' because it doesn't really return trip objects; it returns pairs of transitions that represent trips.

To follow suit, 'transitionTrip2TripObj' is renamed 'tripTransitions2UnprocessedTrip'.

Added a bit of JSDoc to help clarify what these functions do.
Filled in more trip object types, and specifically some literal types that define unprocessed trips. (like "source" will always be 'unprocessed').

CompTripLocations changed from `number[]` for coordinates to geojson's `Position`, since that is more descriptive. Renamed CompTripLocations to CompositeTripLocation (this type represents only 1 location).

Used the CompositeTripLocation type in timelineHelper.
in locations2GeojsonTrajectory, the return type needed `properties` for it to be considered a Geojson `Feature`.

formattedSectionProperties types as the return type of the function
The array has randomly generated accuracy and RSSI values
Further, when we exit the region, all the results are undefined

This is part of the change to test the data model defined in
e-mission/e-mission-docs#1062 (comment)
The simulation buttons are white text on a red background
Consistent with the plans in:
e-mission/e-mission-docs#1062 (comment)

add support for storing simulated BLE responses and transitions in the
usercache

Concretely:
- when there is a monitor entry event, we store a `bluetooth_ble` entry with
  `REGION_ENTER`
- when there is a range event, we store a `bluetooth_ble` entry with
  `RANGE_UPDATE`. if we receive three consecutive range updates within 5 minutes, we generate a `BLE_BEACON_FOUND` transition
- when there is a monitor exit event, we store a `bluetooth_ble` entry with `REGION_EXIT` and generate a `BLE_BEACON_LOST` transition

Note that this has some fixes found while testing android:
e-mission/e-mission-docs#1062 (comment)

In addition, the callbacks exposed that the format of the range callback
that we were using earlier was incorrect. The `beacons` array is at the
same level as the `region`

Testing done on both iOS and android:
- Scan for BLE beacons
- Region enter
- Range (3-4 times); see `ble_found` transition
- Region exit; see `ble_lost` transition
We nearly have the Bluetooth integration done. Now we just need the phone to retrieve BLE scans and implement some matching logic to figure out which beacons go to which trips.

in timelineHelper there is a new function to retrieve BLE scans using the unified data loader.

in inputMatcher we have new functions that go through each timelineEntry and match it to a beacon. Then it looks up that beacon (by its major:minor pair) in the app config to get the vehicle identity.

In LabelTab, we have a new map, timelineBleMap, which holds the mapping of timeline entry IDs to vehicle identities.
With the ability to detect vehicles by Bluetooth, we now have 2 ways to determine the mode of a trip: user labels or Bluetooth.
This makes it necessary to refactor and make the code flexible to handle both.

"Confirmed mode" is the new term for a mode that has been determined; either by the user-provided MODE label, or by the vehicle identity determined by Bluetooth.

"Confirmed mode" is given by the "confirmedModeFor" function. Throughout the codebase, we now use `confirmedModeFor(trip)` instead of `labelFor(trip, 'MODE')`.
For existing configurations that use user MODE labels, this does not change the behavior.

-- While doing this I refactored useGeojsonForTrip to directly accept the base mode, since that is what it really needs, instead of the labeledMode. As a result, it also doesn't need labelOptions as an argument anymore.
shankari and others added 3 commits April 13, 2024 14:20
In e-mission/e-mission-data-collection#221
we have added a new method to the data collection interface that allows
us to create mock BLE objects and save them using the native methods.
This allows us to simulate the real world scenario more closely, by
saving from the native code. while still avoiding hacks to real code.

----------------------------------

Android:

```
04-13 13:34:28.204 13563 13563 I chromium:   "state": "CLRegionStateInside"
04-13 13:34:28.204 13563 13563 I chromium: }", source: https://localhost/plugins/cordova-plugin-em-unifiedlogger/www/unifiedlogger.js (49)
04-13 13:34:28.222 13563 13763 I System.out: About to execute query SELECT data FROM userCache WHERE key = 'background/bluetooth_ble' AND type = 'sensor-data' AND write_ts >= 1.713040168E9 AND write_ts <= 1.713040468E9 ORDER BY write_ts DESC
04-13 13:34:28.241 13563 13763 W PluginManager: THREAD WARNING: exec() call to UserCache.getSensorDataForInterval blocked the main thread for 20ms. Plugin should use CordovaInterface.getThreadPool().
04-13 13:34:28.253 13563 31932 D BuiltinUserCache: Added value for key background/bluetooth_ble at time 1.713040468221E9
04-13 13:34:28.273 13563 31932 D BuiltinUserCache: Added value for key background/bluetooth_ble at time 1.713040468254E9
04-13 13:34:28.319 13563 31932 D BuiltinUserCache: Added value for key background/bluetooth_ble at time 1.713040468276E9
04-13 13:34:28.351 13563 31932 D BuiltinUserCache: Added value for key background/bluetooth_ble at time 1.71304046832E9
04-13 13:34:28.371 13563 13563 I TripDiaryStateMachineRcvr: noarg constructor called
04-13 13:34:28.373 13563 31932 D BuiltinUserCache: Added value for key background/bluetooth_ble at time 1.713040468352E9
```

iOS

```
2024-04-13 13:38:22.029076-0700 emission[77938:10285250] DEBUG:[BLE] didRangeBeaconsInRegion
2024-04-13 13:38:22.029320-0700 emission[77938:10285250] DEBUG: [BLE] didRangeBeaconsInRegion
2024-04-13 13:38:22.031065-0700 emission[77938:10285250] DEBUG:{
}
2024-04-13 13:38:25.060123-0700 emission[77938:10285250] data has 174 bytes, str has size 174
2024-04-13 13:38:25.062087-0700 emission[77938:10285250] data has 174 bytes, str has size 174
2024-04-13 13:38:25.063593-0700 emission[77938:10285250] data has 173 bytes, str has size 173
2024-04-13 13:38:25.065085-0700 emission[77938:10285250] data has 175 bytes, str has size 175
2024-04-13 13:38:25.066255-0700 emission[77938:10285250] data has 175 bytes, str has size 175
2024-04-13 13:38:26.343114-0700 emission[77938:10285250] THREAD WARNING: ['DataCollection'] took '4307.931885' ms. Plugin should use a background thread.
2024-04-13 13:38:26.350811-0700 emission[77938:10285250] In TripDiaryStateMachine, received transition T_BLE_BEACON_FOUND in state STATE_ONGOING_TRIP
2024-04-13 13:38:26.350982-0700 emission[77938:10285250] DEBUG: In TripDiaryStateMachine, received transition T_BLE_BEACON_FOUND in state STATE_ONGOING_TRIP
2024-04-13 13:38:26.352531-0700 emission[77938:10285250] data has 92 bytes, str has size 92
2024-04-13 13:38:26.354445-0700 emission[77938:10285250] data has 69 bytes, str has size 69
2024-04-13 13:38:26.355964-0700 emission[77938:10285250] Got unexpected transition T_BLE_BEACON_FOUND in state STATE_ONGOING_TRIP, ignoring
2024-04-13 13:38:26.356106-0700 emission[77938:10285250] Ignoring silent push notification
```
The background/bluetooth_ble data type has "major" and "minor" as decimal integers. (e-mission/e-mission-docs#1062 (comment))
But in the vehicle_identities spec, we have major:minor pairs as hexadecimal strings. So we will need to convert.

decimalToHex handles this and allows us to add padding, ensuring the converted major and minor are always 4 hex characters.
(so 1 becomes "0001", 255 becomes "00ff", etc.)
This feature allows the user to refresh the app configuration without having to log out and back in.
(This will be handy for our alpha "run-through" of the fermata project, e-mission/nrel-openpath-deploy-configs#89.
we will likely need to update beacon + vehicle info periodically.

dynamicConfig.ts
-- in fetchConfig, pass an option to the fetch API so it does not use a cached copy of the config. We want to make sure it's actually checking for the latest config.
-- export function loadNewConfig so it can be used in ProfileSettings.tsx

ProfileSettings.tsx
--add function refreshConfig, which calls loadNewConfig and triggers a hard refresh if the config has changed. Toast messages to guide the user through the process.
--add the new row itself: "Refresh App Configuration" (which also shows the current version of the app config)

appConfigTypes.ts
--add prop 'version' to config type. Every config has this property.

Testing done:
On a local dev environment with locally hosted configs, I was signed into an nrel-commute opcode. I updated the local config file, changing "version" from 1 to 2 and changing "use_imperial" from true to false.
In the UI Profile tab, the new row showed "Current version: 1". After clicking the row, the app reloads and UI now shows 'km' instead of 'miles'. I went back to the Profile tab and the new row now shows "Current version: 2". Clicking the row a second time triggers a toast message saying "Already up to date!"
@JGreenlee
Copy link
Collaborator Author

New row in the Profile tab allows users to update the config without logging out and back in

Simulator Screen Recording - iPhone 8 - 2024-04-14 at 17 58 00

@JGreenlee
Copy link
Collaborator Author

JGreenlee commented Apr 15, 2024

Once I finally started getting background/bluetooth_ble entries to show up, this error was triggered:

image

I looked at the data. Indeed, every single background/bluetooth_ble entry has major and minor of 0, despite those values being defined when they are passed to the mockBLEObjects function.

I looked through the plugin and I think I found the source of the issue.
https://github.com/e-mission/e-mission-data-collection/blob/a83ae18d84ad04946d50ab606022246f777fb692/src/android/wrapper/BluetoothBLE.java#L87

This code snippet uses == to compare strings when it should use equals(). That causes the major and minor (among other properties to never get filled in for the fake ble objects


Created PR e-mission/e-mission-data-collection#223

www/index.html Outdated
@@ -3,7 +3,7 @@
<head>
<meta charset="utf-8">
<meta name="viewport" content="initial-scale=1.0, maximum-scale=1.0, user-scalable=no, width=device-width, viewport-fit=cover">
<meta http-equiv="Content-Security-Policy" content="default-src 'self' data: gap: https://ssl.gstatic.com https://nominatim.openstreetmap.org https://raw.githubusercontent.com; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; img-src 'self' https://tile.openstreetmap.org 'unsafe-inline' data: 'unsafe-eval'">
<meta http-equiv="Content-Security-Policy" content="default-src 'self' data: gap: https://ssl.gstatic.com https://nominatim.openstreetmap.org https://raw.githubusercontent.com http://10.0.2.2:9090; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; img-src 'self' https://tile.openstreetmap.org 'unsafe-inline' data: 'unsafe-eval'">
Copy link
Contributor

Choose a reason for hiding this comment

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

If we add this, we should also add http://localhost:9090. Although I really don't think we should change this in the index.html since it can be a (very farfetched) security hole in production. Ideally, we would add these (and the edit-config tag in config.xml as part of npm run build-dev and remove them as part of npm build.

For now, I have the changes implemented locally to allow testing but have not checked them in.
I also made https://github.com/e-mission/e-mission-phone/pull/1145/files#diff-d7df3a5aedf99d611872de9affe54d6bd62531488711022ee9f33d1ac86baab6R148 locally but didn't check it in 😄

@shankari
Copy link
Contributor

@JGreenlee You should also be able to use the staging app to generate these now since you have real beacons

  • use the geofence exit from the developer zone
  • move the beacon near the device
  • wait until you get real BLE scans
  • log in to the same token from the emulator

@JGreenlee
Copy link
Collaborator Author

Testing locally with "simulated" beacon scans. This was a fake trip (I used "Routes" in the android emulator) and it successfully matched to a vehicle identity ("Jack's Mazda 3").

This vehicle identity was configured with the major and minor of the simulated beacon scans (they are hardcoded as 4567 and 1945 in decimal, which are 11d7 and 0799 in hexadecimal)

  "vehicle_identities": [
    {
      "value": "car_jacks_mazda3",
      "bluetooth_major_minor": ["11d7:0799"],
      "text": "Jack's Mazda 3",
      "baseMode":"CAR",
      "met_equivalent":"IN_VEHICLE",
      "kgCo2PerKm": 0.22031,
      "vehicle_info": {
        "type": "car",
        "license": "JHK ****",
        "make": "Mazda",
        "model": "3",
        "year": 2014,
        "color": "red",
        "engine": "ICE"
      }
    ...
image

@JGreenlee
Copy link
Collaborator Author

JGreenlee commented Apr 15, 2024

Next, just need to update conditional surveys (along with the showsIf expressions in the config) so they can utilize the matched vehicle identity

For the fermata project, the survey shown for a trip should depend on what vehicle was detected from bluetooth.
For unprocessed trips, this information is not going to be in the raw trip object. That's why we have "timelineBleMap", which is kept separate from the raw trip objects.
But to access this while evaluating the conditional survey "showsIf" expressions, we'll need to include more things in the eval's scope.

I decided that it would be appropriate to use our "derived properties" hook for this. we can add a new property to derived properties, then we can include all the derived properties in the scope in which the "showsIf" expressions are evaluated.

This will probably be adjusted later.

Note: I also simplified the type definition of DerivedProperties. It now just uses the return type of the useDerivedProperties hook.
When matching we basically are trying to find the major:minor pair that was sensed the most frequently during the trip.
But should only consider RANGE_UPDATE events for this. The REGION_ENTER and REGION_EXIT events do not have major and minor defined, so we should exclude them for the purpose of matching.

Add a filter condition for this. Also updated function + variable names to make it clearer that we are only considering 'ranging' scans.

Note that before processing, the value of eventType is a string ('RANGE_UPDATE'). But the server changes this to an enum value where 'RANGE_UPDATE' -> 2. So we have to check for both.
JGreenlee added a commit to Abby-Wheelis/nrel-openpath-deploy-configs that referenced this pull request Apr 15, 2024
Finally!

First, the coordinates change from NREL Building 16 (which we were using for the purpose of testing) to DFC Building 40 (which is the actual location of the Fermata chargers).

Second, instead of using the 'sensed_mode_str' (which we were also doing for the purpose of testing), we can actually use the vehicle detected by bluetooth to show surveys conditionally.

These "showsIf" expressions work with the changes in e-mission/e-mission-phone#1145.
It will not work in Python yet; I am planning a way to do that later.

"confirmedMode" is either a vehicle detected by bluetooth, or the label chosen by the user. In this instance it is the former; it is one of the vehicles defined under "vehicle_identities" (or, for a trip where there was no vehicle detected, detected it is undefined).

Updating config version to 3 so we can receive these updates using the in-app config refresh feature!
@JGreenlee JGreenlee marked this pull request as ready for review April 15, 2024 19:35
@JGreenlee
Copy link
Collaborator Author

With the updates to e-mission/nrel-openpath-deploy-configs#91 (now using version 3 of that config):

Surveys are now shown conditionally based on the detected vehicle rather than by the sensed mode. Previously, this trip prompted the EV Roaming survey because the sensed mode was UNKNOWN. Now it prompts the Gas Car survey because the detected vehicle is Jack's Mazda 3, a gas car.

The January trips (test data) don't match a detected vehicle. So no survey is prompted.

@JGreenlee
Copy link
Collaborator Author

@shankari I think it's ready. This PR includes #1141 so keep that in mind if you want to review it separately

@shankari
Copy link
Contributor

I don't have time for a review, and @JGreenlee knows the UI code better than me anyway. So I am going to merge this and create a new staging release, so we can test this afternoon/tonight and submit for app review tonight or tomorrow morning.
I will leave this in "Ready for review" to remind me to review for future cleanup

@shankari
Copy link
Contributor

It looks like code coverage is broken because of a codecov.io issue. We may have to revisit it @jiji14

@shankari shankari merged commit 9cfff85 into e-mission:master Apr 15, 2024
6 of 7 checks passed
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.

2 participants