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

Address validation in MacOS #26

Closed
cyclemaxwell opened this issue Mar 30, 2023 · 7 comments · Fixed by #27
Closed

Address validation in MacOS #26

cyclemaxwell opened this issue Mar 30, 2023 · 7 comments · Fixed by #27
Labels

Comments

@cyclemaxwell
Copy link
Contributor

Bluetooth address validation fails on MacOS when the connect button is pressed. The address returned by the QBluetoothDeviceInfo.address() function is all zeros.

@cyclemaxwell
Copy link
Contributor Author

I've started to debug and made a utility function (in utils.py) that checks the platform and selects whether to request the address or the uuid.

def get_address_or_uuid(sensor):
    """Return MAC (Windows, Linux) or UUID (macOS)."""
    system = platform.system()
    if system in ["Linux", "Windows"]:
        return sensor.address().toString()
    elif system == "Darwin":
        return sensor.deviceUuid().toString()[1:-1]  # slice to strip off curly braces

This replaces use of the .address() method.

However, this regex:

"[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$"

does not match with the uuid of my polar H10.
The culprit seems to be this part: [089ab][0-9a-f]{3} (second to last grouping between dashes).
Is there a document that this particular regex comes from? I assume it's trying to validate that it's a Polar device?

@JanCBrammer
Copy link
Owner

I suspect I took the regex from this SO post (can't say for sure though): https://stackoverflow.com/questions/7905929/how-to-test-valid-uuid-guid.

The regex should capture UUIDs according to the RFC 4122 specification, which should be compatible with the format that Apple devices seem to use when interacting with Bluetooth peripherals.

@JanCBrammer
Copy link
Owner

checks the platform and selects whether to request the address or the uuid

Nice. Posting the respective Qt docs here for future reference: https://doc.qt.io/qt-6/qbluetoothdeviceinfo.html#address

@JanCBrammer
Copy link
Owner

JanCBrammer commented Mar 30, 2023

This replaces use of the .address() method.

The call to .address() needs to be replaced here as well then?
https://github.com/search?q=repo%3AJanCBrammer%2FOpenHRV%20.address()&type=code

Calling remoteAddress() on the QLowEnergyController probably also fails on MacOS:

return self.client.remoteAddress().toString()

See https://doc.qt.io/qt-6/qlowenergycontroller.html#remoteAddress vs https://doc.qt.io/qt-6/qlowenergycontroller.html#remoteDeviceUuid

Looks like it'd be good to centralize the logic that handles the platform-specific sensor identification (per MAC or UUID).

@cyclemaxwell
Copy link
Contributor Author

Nice. I guess this means implementing a second utility function get_remote_address_or_uuid(client)

@cyclemaxwell
Copy link
Contributor Author

The regex should capture UUIDs according to the RFC 4122 specification, which should be compatible with the format that Apple devices seem to use when interacting with Bluetooth peripherals.

As far as I can tell, my H10 is returning a non-compliant uuid. (second to last grouping begins with e) of the form:
XXXXXXXX-XXXX-4XXX-eXXX-XXXXXXXXXXXX

Is there a reason to require that the uuid be compliant with standards. Would it be ok to simply require a pseudo-UUID to be formatted with the correct number of hex characters and dashes?

@JanCBrammer JanCBrammer linked a pull request Apr 11, 2023 that will close this issue
@mirkancal
Copy link
Contributor

I'm also on MacOS with Polar H10. I can see my device but can't connect.

Invalid sensor address: cd818013-47f2-e83e-dc7c-05f6a924a5ad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants