sharpbrick / powered-up

.NET implementation of the LEGO PoweredUp Protocol
MIT License
99 stars 19 forks source link

Controlling two hubs not working (Lego 42100) #190

Open ChrisKretschmer opened 1 year ago

ChrisKretschmer commented 1 year ago

Hi all, I'm trying to control the two hubs in the Lego kit 42100 (Liebherr excavator) I tried to follow the example https://github.com/sharpbrick/powered-up/blob/master/examples/SharpBrick.PoweredUp.Examples/ExampleTwoHubsMotorControl.cs

When I try to connect to the two hubs using the BLE-MACs, i get the following error:

System.ArgumentNullException: Value cannot be null. (Parameter 'device')
   at SharpBrick.PoweredUp.WinRT.WinRTPoweredUpBluetoothDevice..ctor(BluetoothLEDevice device)
   at SharpBrick.PoweredUp.WinRT.WinRTPoweredUpBluetoothAdapter.GetDeviceAsync(IPoweredUpBluetoothDeviceInfo bluetoothDeviceInfo)
   at SharpBrick.PoweredUp.Bluetooth.BluetoothKernel.ConnectAsync()
   at SharpBrick.PoweredUp.Protocol.LegoWirelessProtocol.ConnectAsync(SystemType knownSystemType)
   at SharpBrick.PoweredUp.Hub.ConnectAsync()
   at LegoAPI.Services.Lego.Connect() in C:\Code\LegoAPI\LegoAPI\LegoAPI\Services\Lego.cs:line 54
   at LegoAPI.Services.Lego..ctor(PoweredUpHost host) in C:\Code\LegoAPI\LegoAPI\LegoAPI\Services\Lego.cs:line 28
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
(...)

The code looks as follows:

public ulong BluetoothAddressHub1 = BytesToULong(new byte[] { 0x90, 0x84, 0x2b, 0x64, 0x40, 0x3c });
public ulong BluetoothAddressHub2 = BytesToULong(new byte[] { 0x90, 0x84, 0x2b, 0x64, 0x85, 0x95 });

private async Task Connect()
{
Hub = await _host.CreateByStateAsync<TechnicMediumHub>(BluetoothAddressHub1);
await Hub.ConnectAsync();
Hub2 = await _host.CreateByStateAsync<TechnicMediumHub>(BluetoothAddressHub2);
await Hub2.ConnectAsync();
(...)

Connecting to a single hub works fine using the discoverAsync methods...

Does anyone has a idea?

Thanks in advance!

mennolodder commented 9 months ago

This probably is the same issue I ran into here when trying the raw protocol: https://github.com/sharpbrick/powered-up/issues/58#issuecomment-1883782361

mennolodder commented 8 months ago

I fixed this, the problem was that the BlueGigaBLEHelper.ByteArrayToUlong does not give the right result (for WinRT).

I used the value that I got from running poweredup device list, which did work.

Edit: I see that in the latest version this command also nicely formats the adress, so that is no longer a work around.

mennolodder commented 8 months ago

I found why I got the wrong address from BlueGigaBLEHelper.ByteArrayToUlong .

My hub has the following address (taken from the currently released version device list command):

string address = "90:84:2B:63:D2:B5"
ulong address = 158897338045109;
 Convert.ToUInt64("0x90842b63d2b5", 16); // yields 158897338045109 which I can use to connect to it manually.

Passing this to the BlueGigaBLEHelper class, as many of the manuals suggest gives

ulong adressAsLongConverted = BlueGigaBLEHelper.ByteArrayToUlong(new byte[] { 0x90, 0x84, 0x2b, 0x63, 0xd2, 0xb5 }); // results in 199915211555984

However reversing the bytes gives the expected result

ulong adressReversedAsLongConverted = BlueGigaBLEHelper.ByteArrayToUlong(new byte[] { 0xb5, 0xd2, 0x63, 0x2b, 0x84, 0x90}); // results in 158897338045109

As inspecting the helper class code shows, BlueGigaBLEHelper takes the left byte (index 0 in the array) as the least significant. Where as the other implementations use the most significant byte.

At the very least this deserves some explanation and updating of the examples and readme. But if the BlueGigaBLE lib actually expects the byte in the reversed order then we should probably add abstraction for that difference as well. Or is the BlueGigaBLEHelper implementation just wrong and should we fix that?

On a sidenote I think the conversion function is generically useful and we should also make it available for the WinRT library.

mennolodder commented 7 months ago

@tthiery Have you had a chance to look at this? I can make a PR, just not sure what direction is the right one (is it that the libraries are different, or does the other one also contain a bug). Probably some class to help with different forms of addresses.

tthiery commented 7 months ago

I would be very happy if you provide a PR on the topic. It should be easy to use the text output from discovery, debug, logging, etc and c&p into static code/config. It just should be uniform. Regards BlueGiga, I do not have the bluetooth adapter, so I can not add anything to that.

Thanks for all your contributions @mennolodder

mennolodder commented 7 months ago

Ok i'm going to do the following

@tthiery can you assign this to me?