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

Discuss BLE Interface, BLE MAC Address and general Selection Handling #154

Closed
3 of 5 tasks
tthiery opened this issue Mar 31, 2021 · 15 comments · Fixed by #173
Closed
3 of 5 tasks

Discuss BLE Interface, BLE MAC Address and general Selection Handling #154

tthiery opened this issue Mar 31, 2021 · 15 comments · Fixed by #173

Comments

@tthiery
Copy link
Member

tthiery commented Mar 31, 2021

Other?

@dkurok @vuurbeving @Berdsen

@tthiery tthiery added this to the v4.0 (breaking) milestone Mar 31, 2021
@tthiery tthiery self-assigned this Mar 31, 2021
@tthiery tthiery removed their assignment Mar 31, 2021
@tthiery tthiery changed the title Discuss BLE MAC Address and general Selection Handling Discuss BLE Interface, BLE MAC Address and general Selection Handling Mar 31, 2021
@dkurok
Copy link
Contributor

dkurok commented Mar 31, 2021

Maybe a
Task<bool>Configure(Dictionary<string , string>)
in the IPoweredUpBluetoothAdapter for configuring an adapter if needed (for example for the BlueGiga I have to configure at least the COM-port it is working on; Trace-options or alike can also be configured over that).
Can simply retrun true for non-configurable adapters. Should be called very early :-)

@dkurok
Copy link
Contributor

dkurok commented Mar 31, 2021

iOS not exposing the MAC-adress of a BLE device:
An iOS-adapter has to connect to a filtered (by LEGO-ServiceUUID) LEGO-Hub first and request the primary MAC-address (property 0x=D, which is not changeable according to LWP 3.0, chapter 3.5.1; see also 3.5.5: only upstream); change this into the format needed internally for uniquly identifiying a Hub (ulong actually) and then disconnect (until further calls from the library do a connect). All that in IPoweredUpBluetoothAdapter.Discover()!
When IPoweredUpBluetoothAdapter.GetDeviceAsync(ulong bluetoothAddress) is called, it has to look for adverting-packets, connect to the first device it reaches (filtered again by LEGO-service), request the primary MAC-adress and if this matches the bluetoothAddress it can stay with that connection to the device; otherwise go for the next advertising Hub.
This is the way Apple works with BLE! Even though the advertising packets have the MAC-address in, Apple's APIs will not expose them; every BLE-device must have a somehow unique identifier. They have their reasons: Under security aspects each device can (and sometimes must) change their MAC-address; some do it regulary on a timer-base. And a using client must have deep information about the device and obtain a still unique identifier (serial-number or alike). The problem is that Apple doesn't give an option here! But the way described above should work, because we can ask the LEGO-Hubs for their "unique" (unchangeable) MAC-address. So this is more a problem of the iOS-implementation of IPoweredUpBluetoothAdapter.
Note: The property-request retruns the MAC-address of the LEGO-device in BIG endian; in the advertisment-packets the MAC-address is always little-endian;so be careful about order :-)

@tthiery
Copy link
Member Author

tthiery commented Mar 31, 2021

Regards Configuration: Using a Method

Yes. That is the way to go. There is Microsoft.Extensions.Configuration component which is essentially a key/value pair and has adapters to json files, command line parsing etc.

using Microsoft.Extensions.Configuration;

public interface IPoweredUpBluetoothAdapter
{
    // ...
    Task ConfigureAsync(IConfiguration configuration);
    // ...
}

In the CLI or other systems we would then have a situation that ...

// ✅ get your config from somewhere
var configuration = new ConfigurationBuilder()
    .AddInMemoryCollection(
        new Dictionary<string, string>
        {
            ["BlueGigaCOMPort"] = "COM4",
            ["BlueGigaTracing"] = bool.TrueString,
        })
       .Build();

// ✅ globally configure the adapter
var adapter = serviceProvider.GetService<IPoweredUpBluetoothAdapter>();
await adapter.ConfigureAsync(configuration); // optionally

// ✅ for each active BLE device/hub to talk to ...
using var scope = serviceProvider.CreateScope(); // use the same adapter
var scopedServiceProvider = scope.ServiceProvider;

// 🕵️‍♀️ that is a fish to grill as well ... but as part of the iOS selection issue.
scopedServiceProvider.GetService<BluetoothKernel>().BluetoothAddress = bluetoothAddress;

// ✅ start using it
var protocol = scopedServiceProvider.GetService<ILegoWirelessProtocol>();

Thumbs Up for this is accepted solution for that problem

@tthiery
Copy link
Member Author

tthiery commented Mar 31, 2021

Regards Configuration: Using the Options Pattern

No interface change, only implementations need to add a Option to the constructor

class BlueGigaAdapter
{
    public BlueGigaAdapter(IOption<BlueGigaOption> option)
   // ...
}

Invocation is less service provider style (anti-pattern) and more DI like.

// ✅ get your config from somewhere
var configuration = new ConfigurationBuilder()
    .AddInMemoryCollection(
        new Dictionary<string, string>
        {
            ["BlueGigaCOMPort"] = "COM4",
            ["BlueGigaTracing"] = bool.TrueString,
        })
       .Build();

// ✅ Init Service Provider including config
var serviceProvider = new ServicesCollection()
    .AddPoweredUp()
    .AddBlueGigaBLE()
    .Configure<BlueGigaOptions>(configuration)
    .Build()

// ✅ for each active BLE device/hub to talk to ...
using var scope = serviceProvider.CreateScope(); // use the same adapter
var scopedServiceProvider = scope.ServiceProvider;

// 🕵️‍♀️ that is a fish to grill as well ... but as part of the iOS selection issue.
scopedServiceProvider.GetService<BluetoothKernel>().BluetoothAddress = bluetoothAddress;

// ✅ start using it
var protocol = scopedServiceProvider.GetService<ILegoWirelessProtocol>();

Thumbs Up for this. Personally I like this more.

@dkurok
Copy link
Contributor

dkurok commented Mar 31, 2021

Regards

Primary interface on the BLE abstractions (ulong vs ?)

What is about an abstraction of this like a BLEIdentifier, which can be constructed by a MAC-Address (little or big-endian over a bool-property) or a ulong or a String like "90:84.2B:4C:FF:F0" (or other format like . or - instead of ":") and then internally use a ulong? Also the byte[6] array could be used (but makes little more effort, because byte[] doesn't implement IEquatable or IEqualityComparer) - the class itself must be usable as a TKey in IDictionary<TKey, TValue> (e.g. for Dictionaries based on the BLEIdentifier). Even better would be an internal Hash derived from the value given to the constructor.
This would for example also allow to use the Advertising Name (property 0x01 of LWP) as an identifier and combined with a property KindOfIdentifier being Address, PrimaryMACAdressProperty or FriendlyName (extensible if wanted; but I don't see no more unique property in LWP) the IPoweredUpBluetoothAdapter.GetDeviceAsync(BLEIdentifier) could decide its way for detecting / connecting to the device.
Not fully thought on all aspects; just an idea...

@tthiery
Copy link
Member Author

tthiery commented Mar 31, 2021

No idea whether it is feasible on iOS but the idea sound good. Would fix all problems at once.

The class PoweredUpBluetoothDeviceInfo is already half of it. Maybe we can create a IPoweredUpBluetoothDeviceInfo which has multiple implementations (one for iOS, one others). That would circle well between IPoweredUpBluetoothAdapter.Discover callback and IPoweredUpBluetoothAdapter.GetDeviceAsync.

For the cases we have an externally provided fixed address, we could construct these explicitly. That explicit construction, I have no idea how it would work on iOS.

@dkurok
Copy link
Contributor

dkurok commented Mar 31, 2021

As said before: Apple has its very own way to deal with BLE.
But also with the idea above, the iOS-implementation must do its own way when using MAC-adress; but the idea doesn't make this harder...
BTW: When using FriendlyName, this can also be handled during advertisment. The Lego-Hub sends this during advertisment. Could be very interesting for someone with many Hubs (trains for example; user can name their Hubs according to locomotive ("BR-51", "Crocodile" or alike))

@dkurok
Copy link
Contributor

dkurok commented Mar 31, 2021

Regards Configuration: Using the Options Pattern
I like this more becasue its nearer to the way it works in Blazor / ASP.NET Core by using DI.

@Berdsen
Copy link
Contributor

Berdsen commented Mar 31, 2021

@dkurok is right about Apple's way to deal the mac and ble.
But do we really require a Bluetooth MacAddress?

Perhaps when we're discovering devices in the BluetoothAdapter discovery we construct the PoweredUpBluetoothDeviceInfo where we assign an identifier and perhaps an BT stack depenendent object.

When then GetDeviceAsync() is called, we just give in this construct, so that the current bt adapter has to handle this by its own implementation:

public async Task<IPoweredUpBluetoothDevice> GetDeviceAsync(PoweredUpBluetoothDeviceInfo deviceInfo)
{
    var device = deviceInfo.NativeDevice as IDevice; // Xamarin
    // var device = deviceInfo.NativeDevice as BluetoothLEDevice // WinRT
}

The bad thing here would be, that a direct connection via known BT MacAddress would not work this way, so this must be handled on a different way.

@tthiery
Copy link
Member Author

tthiery commented Mar 31, 2021

That is exactly my thought. Therefore the idea of the interface. The Mac address can then be a detail only a few implentations will expose. iOS object will then carry whatever information.

@tthiery
Copy link
Member Author

tthiery commented Mar 31, 2021

Primary Device Identity Interface

Since iOS is abstracting the identity of the BLE device, each stack should expose its own identity object. Therefore I propose the following changes

Addition of a new interface

public interface IPoweredUpBluetoothDeviceInfo
{
    byte[] ManufacturerData { get; }
    string Name { get; set; }
}
public interface IPoweredUpMacAddress
{
    byte[] MacAddress { get; }
}

public class WinRTDeviceInfo : IPoweredUpBluetoothDeviceInfo, IPoweredUpMacAddress { }
public class XamarinOnIOSDeviceInfo : IPoweredUpBluetoothDeviceInfo { }
public class XamarinOnAndroidDeviceInfo : IPoweredUpBluetoothDeviceInfo, IPoweredUpMacAddress { }
public class BlueGigaDeviceInfo : IPoweredUpBluetoothDeviceInfo, IPoweredUpMacAddress { }

Change to the existing Adapter interface

Use the new interface

public interface IPoweredUpBluetoothAdapter
{
    void Discover(Func<IPoweredUpBluetoothDeviceInfo, Task> discoveryHandler, CancellationToken cancellationToken = default);

    Task<IPoweredUpBluetoothDevice> GetDeviceAsync(IPoweredUpBluetoothDeviceInfo deviceInfo);
}

public class BluetoothKernel
{
    // ...
    // remove: public ulong BluetoothAddress { get; set; }
    public IPoweredUpBluetoothDevice BluetoothDeviceInfo { get; set; }
}

Finally
Change all the interfaces using the ulong bluetooth addresses.

I like the change. Is that working for team iOS? Must be fixed in v4 timeline. Thumbs Up?

@dkurok
Copy link
Contributor

dkurok commented Apr 1, 2021

regards Primary Device Identity Interface:
Why do you want to split it into two interfaces here? The byte[] MacAddress could go directly into IPoweredUpBluetoothDeviceInfo IMHO. At some point in implementation EVERY BLE-stack will need it to uniquly identify a LWP-device/Hub by its MAC-address (getting it over advertisment or in case of iOS over connect & request property 0x0D) , because the LWP-Hubs don't have any other really unique value (serial# or alike) actually. And beeing prepared that something like that may change (so some other kind of unique identifier inside the hubs; may come with firmware-updates) we can take any object instead of byte[] as long as it is unique.
Alternative Suggestion:

public interface IPoweredUpBluetoothDeviceInfo: IEquatable<IPoweredUpBluetoothDeviceInfo>
{
    byte[] ManufacturerData { get; }
    string Name { get; set; }
    object UniqueIdentifier { get; }
    
    override bool Equals(IPoweredUpBluetoothDeviceInfo obj)
    {
        if (obj == null || GetType() != obj.GetType())
        {
            return false;
        }
        //here there may be some change needed depending on what concrete type is used in the real implementation
        //byte[] for example must override it, because byte[] doensn't adhere to IComparable/IEquatable
        if (this.UniqueIdentifier.Equals(obj.UniqueIdentifier))
        {
            return true;
        }
    }
    
    // override object.GetHashCode
    public override int GetHashCode()
    {
        return UniqueIdentifier.GetHashCode();
    }  
}

Rest is nearly identical to above suggestion from @tthiery (just leave out the IPoweredUpMacAddress)

@rickjansen-dev
Copy link
Contributor

I do agree with you on making the thing IEquatable, mainly for dictionary support. But that's only if we step away from using ulong as the unique identifier of course (which is probably a good thing).

I'm not fond of it being an object maybe, it seems to generic/broad in a definition. You really need to be aware that whatever you are using as the unique identifier should be properly implementing equals and gethashcode (but that might be a very reasonable assumption/choice though).
You are simply relaying the uniqeness from the interface to the object. You could have a dictionary with the uniqueidentifier as Tkey instead everywhere, might not be as pretty, but it would also work.

Just a suggestion: maybe we can replace object by a UniqueIdentifer class that contains methods to convert from (and to) various mac address representations (eg FromMacAddressLong(), FromMacAddressString()) and ios device info (FromIOSDeviceRepresentation()). If and only if we can agree on a common storage for a unique identifier (mac address or otherwise), including iOS. From* and To* methods could maybe be extension methods so they can reside in specific implementation binaries/namespaces.

@Berdsen
Copy link
Contributor

Berdsen commented Apr 16, 2021

Just a small update. I finally made a working setup to be able to deploy to some Apple HW.
It's really tough without having an own mac or an apple developer account but I'm finally able to deploy to a real device.
The next days I would try to look into what's happening with the Xamarin BLE adapter for iOS and perhaps make it in the first step like it is done in sharpbrick/example-xamarin#1 and commit it also to this PR.

tthiery added a commit that referenced this issue Apr 30, 2021
- Abstract identity in an interface independent of Mac Address
- Change PoweredUpHost function to use generic state instead
  of bluetooth address as ulong
- Implement interface in existing adapters (simple transformation)
- Change BluetoothKernel interface to accept device instead of
  bluetooth address
- Modify existing examples

#154 breaking
tthiery added a commit that referenced this issue Apr 30, 2021
@tthiery
Copy link
Member Author

tthiery commented Apr 30, 2021

Hi Friends ... can you have a look at the PR #173 I created. It is the implementation of the Device Identification Abstraction.

Further, there is a detail about Mac Address stringification where I need your thought power. I am not sure if all our adapters have they ulong mac address we used so far in the same way (regards byte order etc). Also not regards little and big endianess.

@Berdsen @vuurbeving @dkurok

tthiery pushed a commit that referenced this issue May 8, 2021
- Added device identifier for iOS
- Reworked device identifier for Xamarin

#154
tthiery added a commit that referenced this issue May 8, 2021
- Abstract identity in an Bluetooth Device independent of Mac Address
- Change PoweredUpHost function to use generic state instead
  of bluetooth address (as ulong)
- Implement interface in existing adapters (simple transformation)
- Change BluetoothKernel interface to accept device info abstraction
   instead of bluetooth address
- Modify existing examples
- Add formatting to the MAC Address DeviceInfo
- Adjust Xamarin and iOS device identifiers (#176)

#154

Co-authored-by: Berdsen <berdsen.home@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants