Skip to content

Conversation

thebookins
Copy link
Contributor

@thebookins thebookins commented Feb 15, 2018

This PR

  • updates the example app to match recent changes to xDripG5
  • fixes some warnings when pod install is called (see here for reference)
  • adds a setter for ID in Transmitter
  • fixes a deprecation warning
  • updates iOS version in xDripG5.podspec to 10.3 to keep dependencies happy

Note that the compilation order for xDripG5 needs to be changed as a workaround for https://bugs.swift.org/browse/SR-631. Specifically, PeripheralManager+G5.swift and BluetoothServices.swift need to be compiled after PeripheralManager.swift to avoid errors like "'Configuration' is not a member type of 'PeripheralManager'". Not sure how to specify compilation order in CocoaPods.

get {
return id.id
}
set(newID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a public setter to Transmitter.id isn’t the right solution, since additional state on Transmitter like activationDate need to be reset. I think it’s cleaner for clients to recreate Transmitter when the ID changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @ps2, will do, much cleaner. I have changed the client code to recreate Transmitter; it just doesn't always start scanning reliably after a change to the transmitter ID. Once I've chased that down I'll push the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there anything I need to do in the client to tear down the old transmitter and replace it with a new one? When I try this, and change the ID in the client, the CBCentralManager state is sometimes .poweredOn, which is all good, but sometimes .unsupported, in which case we can't scan. Does the central manager need to be a singleton or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we store the transmitter state (activationDate and lastTimeMessage in the TransmitterID object, which gets renewed on a new ID). If we write the setter like this

        set(newID) {
            stopScanning()
            id = TransmitterID(id: newID)
            resumeScanning()
        }

then we scan afresh when the user changes ID.

I'd be happy to replace the entire Transmitter object, but can't find a clean way for the old Transmitter object to release all bluetooth resources in time for the new object to use them (if that makes sense).

Copy link
Contributor

Choose a reason for hiding this comment

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

We do need to reset the bluetooth peripheral if we're changing the transmitter id.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #70; I've incorporated some of the things you've been pointing out there. I think this PR should just focus on getting the example app building. I.e. Removing cocoapods, and incorporating the example app into the project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 thanks!

@thebookins
Copy link
Contributor Author

Resolved with #71. Thanks @ps2.

@thebookins thebookins closed this Feb 19, 2018
@thebookins thebookins deleted the example branch May 12, 2018 11:24
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