-
Notifications
You must be signed in to change notification settings - Fork 114
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
Simulate BLE scans from the phone UI #1140
Conversation
shankari
commented
Mar 29, 2024
- Makes it easier to test how BLE works
- Makes it easier to test the plumbing end to end in the emulator
Without this change, we get an error ``` Warning: Each child in a list should have a unique "key" prop. Check the render method of `BluetoothCardList`. See https://reactjs.org/link/warning-keys for more information. BluetoothCard@ionic://localhost/dist/bundle.js:21253:18 BluetoothCardList@ionic://localhost/dist/bundle.js:21666:21 ``` With this change, the error does not occur
We currently have two hardcoded beacons in the list. But everybody is going to have their own beacon for testing. Let's make it possible to add new UUIDs Changes: - New field to enter the UUID - Add button - When add button is clicked, add new entry with the specified UUID and dummy values Testing done: Entered two separate UUIDs and clicked "Add" Two new entries were created in the list
- Creates a new set of fields at the botton of the page for the UUID, identifier, major and minor fields - Adds a new "Add" button - When the "Add" button is pressed, we add a new UUID entry to the default list - After the new entry is added, we clear out the old values to prepare for a new entry Testing done: Added three new entries. They were displayed in the list e-mission/e-mission-docs#1062 (comment)
e-mission/e-mission-docs#1062 (comment) Testing done: - Added values with and without a major and minor value e-mission/e-mission-docs#1062 (comment)
Instead of the statically coded name/identifier This involved changing the text size so that we could see the UUID I wonder if we should make the UUID the subtitle and the major:minor the title Testing done: e-mission/e-mission-docs#1062 (comment)
We don't know what the result will look like when there are multiple beacons with the same UUID, so we just stringify and display the result. We cannot test this without having an actual BLE beacon, so I have added a "fake callback button to simulate a callback" Changes: - stringify the result in `didDetermineStateForRegion` and pass it in to `setRangeStatus` - Change the BluetoothCard to display the `device.result` as Card.Content - Add a new button to fake a callback for a device by getting the delegate and invoking the `didDetermineStateForRegion` method on it with the device - Remove the existing result in the device before invoking the callback to avoid nested results Testing done: e-mission/e-mission-docs#1062 (comment)
@JGreenlee I would appreciate a review of this change since it is my first ever RN change. You are clearly more skilled than me in the e-mission UI at this point. However, in order to unblock us on the BLE work (notably to test e-mission/e-mission-docs#1062 (comment)), I plan to merge this tonight and create a new staging release. I will address your comments in a cleanup PR. |
The testing results are in e-mission/e-mission-docs#1062 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1140 +/- ##
=======================================
Coverage 77.49% 77.49%
=======================================
Files 29 29
Lines 1693 1693
Branches 370 370
=======================================
Hits 1312 1312
Misses 381 381
Flags with carried forward coverage won't be shown. Click here to find out more. |
Changes: - in the monitor callback, launch the range call with the UUID that was found - change the fake callback to call the range callback 500ms after the merge callback - store the two results separately - display the two results separately using different background colors