Skip to content

Commit

Permalink
Fix duplicated delivery of Omron Measurements (#9)
Browse files Browse the repository at this point in the history
# Fix duplicated delivery of Omron Measurements

## ♻️ Current situation & Problem
This PR fixes some minor issues with the current implementation of Omron
devices. StanfordSpezi/SpeziBluetooth#46 made
two important changes to improve the overall reliability of the
implementation. First, it fixes an issue with notify-only
characteristics (like weight and blood pressure measurement
characteristics) where the `onChange` would not be called. Secondly,
while the device is subscribing to characteristic notifications, the
state is still set to `connecting`, this allows us to properly filter
the initial measurement notifications that Omron devices send when
subscribing to characteristics. Further, this allowed us to fix an issue
where we couldn't properly determine the point in time where a weight
scale was fully paired by the user.

Additionally this PR adds a small time label for the
`MeasurementsRecordedSheet` and adds German localization.

## ⚙️ Release Notes 
* Fixed duplicated deliver of measurements for Omron devices.
* Fixed pairing with Omron weight scales.
* Added a time label to the `MeasurementsRecordedSheet`.
* Added German localization.

## 📚 Documentation
Added a small documentation article that collects knowledge about Omron
devices.


## ✅ Testing
We used manual testing with real Omron devices to verify the
implementation. Some additional unit tests were added to ensure no
regression occur.


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
  • Loading branch information
Supereg authored Aug 16, 2024
1 parent 6fc18b2 commit 16075cb
Show file tree
Hide file tree
Showing 17 changed files with 628 additions and 66 deletions.
14 changes: 7 additions & 7 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ let package = Package(
.library(name: "SpeziOmron", targets: ["SpeziOmron"])
],
dependencies: [
.package(url: "https://github.com/apple/swift-collections.git", from: "1.1.1"),
.package(url: "https://github.com/StanfordSpezi/SpeziFoundation", from: "1.1.1"),
.package(url: "https://github.com/StanfordSpezi/Spezi.git", from: "1.4.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziViews.git", from: "1.5.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziBluetooth", exact: "3.0.0-beta.2"),
.package(url: "https://github.com/apple/swift-collections", from: "1.1.1"),
.package(url: "https://github.com/StanfordSpezi/SpeziFoundation", from: "2.0.0-beta.1"),
.package(url: "https://github.com/StanfordSpezi/Spezi", from: "1.7.1"),
.package(url: "https://github.com/StanfordSpezi/SpeziViews", from: "1.5.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziBluetooth", from: "3.0.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziNetworking", from: "2.1.1"),
.package(url: "https://github.com/StanfordBDHG/XCTestExtensions.git", .upToNextMinor(from: "0.4.12"))
.package(url: "https://github.com/StanfordBDHG/XCTestExtensions", .upToNextMinor(from: "0.4.12"))
] + swiftLintPackage(),
targets: [
.target(
Expand Down Expand Up @@ -135,7 +135,7 @@ func swiftLintPlugin() -> [Target.PluginUsage] {

func swiftLintPackage() -> [PackageDescription.Package.Dependency] {
if ProcessInfo.processInfo.environment["SPEZI_DEVELOPMENT_SWIFTLINT"] != nil {
[.package(url: "https://github.com/realm/SwiftLint.git", .upToNextMinor(from: "0.55.1"))]
[.package(url: "https://github.com/realm/SwiftLint.git", from: "0.55.1")]
} else {
[]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,15 @@ extension BloodPressureMeasurement {
extension HKCorrelation {
/// Retrieve a mock blood pressure sample.
@_spi(TestingSupport) public static var mockBloodPressureSample: HKCorrelation {
let measurement = BloodPressureMeasurement(systolic: 117, diastolic: 76, meanArterialPressure: 67, unit: .mmHg, pulseRate: 68)
let dateTime = DateTime(from: .now)
let measurement = BloodPressureMeasurement(
systolic: 117,
diastolic: 76,
meanArterialPressure: 67,
unit: .mmHg,
timeStamp: dateTime,
pulseRate: 68
)
guard let sample = measurement.bloodPressureSample(source: nil) else {
preconditionFailure("Mock sample was unexpectedly invalid!")
}
Expand All @@ -96,7 +104,15 @@ extension HKCorrelation {
extension HKQuantitySample {
/// Retrieve a mock heart rate sample.
@_spi(TestingSupport) public static var mockHeartRateSample: HKQuantitySample {
let measurement = BloodPressureMeasurement(systolic: 117, diastolic: 76, meanArterialPressure: 67, unit: .mmHg, pulseRate: 68)
let dateTime = DateTime(from: .now)
let measurement = BloodPressureMeasurement(
systolic: 117,
diastolic: 76,
meanArterialPressure: 67,
unit: .mmHg,
timeStamp: dateTime,
pulseRate: 68
)
guard let sample = measurement.heartRateSample(source: nil) else {
preconditionFailure("Mock sample was unexpectedly invalid!")
}
Expand Down
15 changes: 12 additions & 3 deletions Sources/SpeziDevices/HealthMeasurements.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public final class HealthMeasurements: @unchecked Sendable {
/// To clear pending measurements call ``discardMeasurement(_:)``.
@MainActor public private(set) var pendingMeasurements: [HealthKitMeasurement] = []

@Dependency @ObservationIgnored private var bluetooth: Bluetooth?
@Dependency(Bluetooth.self) @ObservationIgnored private var bluetooth: Bluetooth?

private var modelContainer: ModelContainer?

Expand Down Expand Up @@ -138,8 +138,8 @@ public final class HealthMeasurements: @unchecked Sendable {
}


Task.detached { @MainActor in
self.fetchMeasurements()
Task.detached {
await self.fetchMeasurements()
}
}

Expand All @@ -158,6 +158,11 @@ public final class HealthMeasurements: @unchecked Sendable {
guard let self, let device else {
return
}
guard case .connected = device.state else {
logger.debug("Ignored weight measurement that was received while connecting: \(String(describing: measurement))")
return
}

let service = device[keyPath: keyPath]

Check warning on line 166 in Sources/SpeziDevices/HealthMeasurements.swift

View workflow job for this annotation

GitHub Actions / CodeQL / Test using xcodebuild or run fastlane

capture of 'keyPath' with non-sendable type 'HealthMeasurements.WeightScaleKeyPath<Device>' (aka 'KeyPath<Device, WeightScaleService>') in a `@Sendable` closure

Check warning on line 166 in Sources/SpeziDevices/HealthMeasurements.swift

View workflow job for this annotation

GitHub Actions / Build and Test iOS / Test using xcodebuild or run fastlane

capture of 'keyPath' with non-sendable type 'HealthMeasurements.WeightScaleKeyPath<Device>' (aka 'KeyPath<Device, WeightScaleService>') in a `@Sendable` closure

Check warning on line 166 in Sources/SpeziDevices/HealthMeasurements.swift

View workflow job for this annotation

GitHub Actions / Build and Test Swift Package iOS / Test using xcodebuild or run fastlane

capture of 'keyPath' with non-sendable type 'HealthMeasurements.WeightScaleKeyPath<Device>' (aka 'KeyPath<Device, WeightScaleService>') in a `@Sendable` closure
logger.debug("Received new weight measurement: \(String(describing: measurement))")
handleNewMeasurement(.weight(measurement, service.features ?? []), from: device.hkDevice)
Expand All @@ -180,6 +185,10 @@ public final class HealthMeasurements: @unchecked Sendable {
guard let self, let device else {
return
}
guard case .connected = device.state else {
logger.debug("Ignored blood pressure measurement that was received while connecting: \(String(describing: measurement))")
return
}
let service = device[keyPath: keyPath]

Check warning on line 192 in Sources/SpeziDevices/HealthMeasurements.swift

View workflow job for this annotation

GitHub Actions / CodeQL / Test using xcodebuild or run fastlane

capture of 'keyPath' with non-sendable type 'HealthMeasurements.BloodPressureKeyPath<Device>' (aka 'KeyPath<Device, BloodPressureService>') in a `@Sendable` closure

Check warning on line 192 in Sources/SpeziDevices/HealthMeasurements.swift

View workflow job for this annotation

GitHub Actions / Build and Test iOS / Test using xcodebuild or run fastlane

capture of 'keyPath' with non-sendable type 'HealthMeasurements.BloodPressureKeyPath<Device>' (aka 'KeyPath<Device, BloodPressureService>') in a `@Sendable` closure

Check warning on line 192 in Sources/SpeziDevices/HealthMeasurements.swift

View workflow job for this annotation

GitHub Actions / Build and Test Swift Package iOS / Test using xcodebuild or run fastlane

capture of 'keyPath' with non-sendable type 'HealthMeasurements.BloodPressureKeyPath<Device>' (aka 'KeyPath<Device, BloodPressureService>') in a `@Sendable` closure
logger.debug("Received new blood pressure measurement: \(String(describing: measurement))")
handleNewMeasurement(.bloodPressure(measurement, service.features ?? []), from: device.hkDevice)
Expand Down
10 changes: 10 additions & 0 deletions Sources/SpeziDevices/Measurements/HealthKitMeasurement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ public enum HealthKitMeasurement {
case weight(HKQuantitySample, bmi: HKQuantitySample? = nil, height: HKQuantitySample? = nil)
/// A blood pressure correlation with an optional heart rate sample.
case bloodPressure(HKCorrelation, heartRate: HKQuantitySample? = nil)

/// The start date of the primary sample
public var startDate: Date {
switch self {
case let .weight(sample, _, _):
sample.startDate
case let .bloodPressure(correlation, _):
correlation.startDate
}
}
}


Expand Down
14 changes: 7 additions & 7 deletions Sources/SpeziDevices/PairedDevices.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ public final class PairedDevices: @unchecked Sendable {

@Application(\.logger) @ObservationIgnored private var logger

@Dependency @ObservationIgnored private var bluetooth: Bluetooth?
@Dependency @ObservationIgnored private var tipKit: ConfigureTipKit
@Dependency(Bluetooth.self) @ObservationIgnored private var bluetooth: Bluetooth?
@Dependency(ConfigureTipKit.self) @ObservationIgnored private var tipKit

private var modelContainer: ModelContainer?

Expand Down Expand Up @@ -163,7 +163,7 @@ public final class PairedDevices: @unchecked Sendable {
}

// We need to detach to not copy task local values
Task.detached { @MainActor in
Task.detached { @Sendable @MainActor in
self.fetchAllPairedInfos()

self.syncDeviceIcons() // make sure assets are up to date
Expand Down Expand Up @@ -294,7 +294,7 @@ public final class PairedDevices: @unchecked Sendable {
}

self.logger.info(
"Detected nearby \(Device.self) accessory\(device.advertisementData.manufacturerData.map { " with manufacturer data \($0)" } ?? "")"
"Detected nearby \(Device.self) accessory\(device.advertisementData.manufacturerData.map { " with manufacturer data \($0.debugDescription)" } ?? "")"
)

discoveredDevices[device.id] = device
Expand Down Expand Up @@ -411,7 +411,7 @@ extension PairedDevices {

try await withThrowingDiscardingTaskGroup { group in
// connect task
group.addTask { @MainActor in
group.addTask { @Sendable @MainActor in
do {
try await device.connect()
} catch {
Expand All @@ -424,7 +424,7 @@ extension PairedDevices {
}

// pairing task
group.addTask { @MainActor in
group.addTask { @Sendable @MainActor in
try await withTaskCancellationHandler {
try await withCheckedThrowingContinuation { continuation in
self.ongoingPairings[id] = PairingContinuation(continuation)
Expand Down Expand Up @@ -664,7 +664,7 @@ extension PairedDevices {

await withDiscardingTaskGroup { group in
for deviceInfo in self._pairedDevices.values {
group.addTask { @MainActor in
group.addTask { @Sendable @MainActor in
guard self.peripherals[deviceInfo.id] == nil else {
return
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SpeziDevices/Testing/MockDevice.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public final class MockDevice: PairableDevice, HealthDevice, BatteryPoweredDevic
@Service public var bloodPressure = BloodPressureService()
@Service public var weightScale = WeightScaleService()

@Dependency private var pairedDevices: PairedDevices?
@Dependency(PairedDevices.self) private var pairedDevices: PairedDevices?

public var isInPairingMode: Bool = false

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,28 @@ import SpeziViews
import SwiftUI


struct TimeSubheadline: View {
private let measurement: HealthKitMeasurement

var body: some View {
Group {
if Calendar.current.isDateInToday(measurement.startDate) {
Text("Today, \(Text(measurement.startDate, style: .time))")
} else {
Text(.now, style: .date) + Text(verbatim: ", ") + Text(.now, style: .time)
}
}
.font(.subheadline)
.foregroundStyle(.secondary)
.lineLimit(1)
}

init(measurement: HealthKitMeasurement) {
self.measurement = measurement
}
}


/// A sheet view displaying one or many newly recorded measurements.
///
/// This view retrieves the pending measurements from the [`HealthMeasurements`](https://swiftpackageindex.com/stanfordspezi/spezidevices/documentation/spezidevices/healthmeasurements)
Expand Down Expand Up @@ -54,6 +76,9 @@ public struct MeasurementsRecordedSheet: View {
Text("Measurement Recorded")
.font(.title)
.fixedSize(horizontal: false, vertical: true)
if let selectedMeasurement {
TimeSubheadline(measurement: selectedMeasurement)
}
} subtitle: {
EmptyView()
} content: {
Expand Down Expand Up @@ -123,11 +148,11 @@ public struct MeasurementsRecordedSheet: View {
return
}

discardSelectedMeasurement(selectedMeasurement)

if measurements.pendingMeasurements.isEmpty {
dismiss()
}

discardSelectedMeasurement(selectedMeasurement)
}
}

Expand Down
Loading

0 comments on commit 16075cb

Please sign in to comment.