[Bug]: IManagedPeripheral.WriteBlob is corrupting the request data.

ryanjkendrick commented 2 years ago

Component/Nuget

BluetoothLE Client (Shiny.BluetoothLE)

What platform(s) are effected?

[ ] iOS 12
[ ] iOS 13
[ ] iOS 14
[X] iOS 15
[ ] Android 8
[ ] Android 9
[ ] Android 10
[ ] Android 11
[ ] Android 12
[ ] UWP (Sponsors ONLY)

Steps To Reproduce

Create new "Hello World" Xamarin Forms Project.
Add latest stable Shiny, Shiny.BluetoothLE and Shiny.BluetoothLE.Hosting packages. Add ReactiveUI.Fody 17.1.46.
Setup a Bluetooth server and start scanning for other devices using the app.
Once connected, serialize an object using ProtoBuf.
Transmit this object from Client to client to server, using IManagedPeripheral.WriteBlob().

An example project can be found here: https://github.com/rhinohq/ShinyBluetoothTest

Expected Behavior

Object should deserialize successfully, demonstrating a successful transfer.

Actual Behavior

ProtoBuf.ProtobufException is thrown.

Exception or Log output

at ProtoBuf.Internal.ThrowHelper.ThrowProtoException (System.String message, System.Exception inner) [0x00000] in //src/protobuf-net.Core/Internal/ThrowHelper.cs:70 at ProtoBuf.ProtoReader.ThrowInvalidField (System.Int32 fieldNumber) [0x00000] in //src/protobuf-net.Core/ProtoReader.cs:317 at ProtoBuf.ProtoReader.SetTag (System.UInt32 tag) [0x0000f] in //src/protobuf-net.Core/ProtoReader.cs:308 at ProtoBuf.ProtoReader+State.ReadFieldHeaderFallback () [0x0003d] in //src/protobuf-net.Core/ProtoReader.State.ReadMethods.cs:714 at ProtoBuf.ProtoReader+State.ReadFieldHeader () [0x00054] in /_/src/protobuf-net.Core/ProtoReader.State.ReadMethods.cs:701 at (wrapper other) System.Object.gsharedvt_out_sig(intptr,intptr) at (wrapper dynamic-method) ShinyBluetoothTest.Models.TestRequest.proto_2(ProtoBuf.ProtoReader/State&,ShinyBluetoothTest.Models.TestRequest) at (wrapper other) System.Object.interp_lmf_mono_interp_entry_from_trampoline(intptr,intptr) at ProtoBuf.Internal.Serializers.SimpleCompiledSerializer1[T].ProtoBuf.Serializers.ISerializer<T>.Read (ProtoBuf.ProtoReader+State& state, T value) [0x00000] in <98f74c6f978b411d80efaccf4d87bd23>:0 at ProtoBuf.ProtoReader+State.ReadAsRoot[T] (T value, ProtoBuf.Serializers.ISerializer1[T] serializer) [0x00031] in <5518f96053c946f49d824de38cc4b388>:0 at ProtoBuf.ProtoReader+State.DeserializeRoot[T] (T value, ProtoBuf.Serializers.ISerializer`1[T] serializer) [0x00013] in <5518f96053c946f49d824de38cc4b388>:0 at ProtoBuf.ProtoReader+State.DeserializeRootImpl[T] (T value) [0x0002b] in <5518f96053c946f49d824de38cc4b388>:0 at ProtoBuf.Serializer.Deserialize[T] (System.IO.Stream source) [0x0000f] in <98f74c6f978b411d80efaccf4d87bd23>:0 at ShinyBluetoothTest.Services.BluetoothService.ReceiveData (System.Object sender, ShinyBluetoothTest.EventArgs.DataReceivedEventArgs args) [0x0000e] in /Users/ryankendrick/GitHub/ShinyBluetoothTest/ShinyBluetoothTest/ShinyBluetoothTest/Services/BluetoothService.cs:106 at ShinyBluetoothTest.Services.BluetoothHostingService.b12_1 (Shiny.BluetoothLE.Hosting.WriteRequest request) [0x0001f] in /Users/ryankendrick/GitHub/ShinyBluetoothTest/ShinyBluetoothTest/ShinyBluetoothTest/Services/BluetoothHostingService.cs:98 at Shiny.BluetoothLE.Hosting.GattCharacteristic.OnWrite (System.Object sender, CoreBluetooth.CBATTRequestsEventArgs args) [0x0005a] in <7b530aa612954fb489c5dae2301a2509>:0 at CoreBluetooth.CBPeripheralManager+_CBPeripheralManagerDelegate.WriteRequestsReceived (CoreBluetooth.CBPeripheralManager peripheral, CoreBluetooth.CBATTRequest[] requests) [0x00011] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/build/ios/native/CoreBluetooth/CBPeripheralManager.g.cs:516 at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr) at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x00013] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:73 at ShinyBluetoothTest.iOS.Application.Main (System.String[] args) [0x00001] in /Users/ryankendrick/GitHub/ShinyBluetoothTest/ShinyBluetoothTest/ShinyBluetoothTest.iOS/Main.cs:17

Code Sample

// Server service setup
this.dataCharacteristic = serviceBuilder.AddCharacteristic(
    BluetoothConstants.DataCharacteristicId,
    cb =>
    {
        cb.SetWrite(request =>
        {
            TestRequest meshMessage;

            using (var stream = new MemoryStream(request.Data))
            {
                meshMessage = Serializer.Deserialize<TestRequest>(stream);
            }

            return GattState.Success;
        });

        cb.SetRead(request => ReadResult.Success(new byte[0]));
        cb.SetNotification();
    }
);

// Client sending code
byte[] data;

using (var stream = new MemoryStream())
{
    var req = new TestRequest() { Data = DateTime.Now.ToString(), Type = TestType.Test };

    Serializer.Serialize(stream, req);

    data = stream.ToArray();
}

managedPeripheral.WriteBlob(
    BluetoothConstants.PeripheralServiceId,
    BluetoothConstants.DataCharacteristicId,
    new MemoryStream(data)
).ToTask();

Code of Conduct

[X] I agree to follow this project's Code of Conduct

aritchie commented 2 years ago

Ok, so first thing is that I don't take bugs with other frameworks around it. However, I'll make a very good guess here: 1) DateTime.Now.ToString() is going to produce more than the standard 20 byte MTU. The purpose of write blob is it loops until the stream is finished thus multiple packets 2) On the server side, you are only processing one packet assuming the stream is finished. It likely isn't.

To debug, you should base64 the bytes on both sides and write it out. You can compare from there. Lastly, take a look at how WriteBlob actually works. It doesn't manipulate the bytes at all, so the only way the stream can get corrupted is

One of the writes fails (which means WriteBlob fails out)
Your RX characteristic isn't set to WriteWithResponse which means bytes could come in out of order

aritchie commented 2 years ago

No feedback on issue

ryanjkendrick commented 2 years ago

Apologies for slow reply, life got in the way.

The issue here was that extra empty data was being added to the packet if data below the size of the MTU was added.

I fixed this by prefixing the length of the data to the start of the transmission. I then knew exactly how much data to expect.

For those in the future that find it useful, an example can be found here: https://github.com/rhinohq/ShinyBluetoothTest/commit/ebf14f9fdf73d07e08c9c4f3d06dc60af47f4426

shinyorg / shiny