›

Cannot stream from route when using "Find" in NetStandard API

FrozenKiwiFrozenKiwi
in Android
Hey,

I've implemented the new 0.3 NetStandard API with a full backend to run on windows/android/iOS.

Things are working fine on Windows, but when running on Android I cannot get the "Find" instruction to deliver me any values.

The following works only on Windows

            _TriggerRoute = await adc.AddRouteAsync(source =>
                source.Find(Threshold.Binary, threshold).Stream(data => SendTrigger(data.Value<byte>(), data.Timestamp))
            );

The following works on both Windows & Android

            _StreamingRoute = await adc.AddRouteAsync(source =>
                source.LowPass(4).Limit(streamPeriod).Stream(data => SendMoisture(data.Value<byte>(), data.Timestamp))
            );

How can this be?  The LowPass + Limit + Stream gives me readings, as expected.  But the Find + Stream never triggers (on android only). 

Otherwise, the two implementations are identical.

Is there anyway to debug the device itself to be sure that the route is setup correctly?

Comments

  • FrozenKiwiFrozenKiwi
    Note - even something as simple as blinking the LED when transmitting data would be super-helpful.  At least then we would know that 1/2 the system is working.  Is there any way to trigger that?
  • EricEric
    edited December 2017
    After setting up the data route, you can dump data processor configs to check that everything was at least correctly setup.  There is no publicly visible SDK method to do this but if you are up for it, you can implement a debug function to the IDataProcessor module that adapts code from the internal pullChainAsync function, specifically these two lines.


    Does the find function correctly work with the previous version of C# SDK?
  • FrozenKiwiFrozenKiwi
    Ok, I figured the problem: 

    It was nothing to do with find, rather it was to do with several functions in the NETStandard API not being async, but calling async functions internally.  In short:

                var gpio = metawear.GetModule<IGpio>();
                gpio.Pins[0].ClearOutput();
                gpio.Pins[2].SetPullMode(MbientLab.MetaWear.Peripheral.Gpio.PullMode.Down);

    Neither of these functions are marked async, but are implemented as so:

                public Task ClearOutput() {
                    return bridge.sendCommand(new byte[] { (byte) GPIO, CLEAR_DO, pin });  // this is an async command
                }

    Which, of course, means that you lose all guarantee's about synchronicity.  I don't know specifically why this caused my issue, but I did note that out-of-order transmits happened, and fixing the async propogation fixed my problem.

    Note: failing to propagate async should throw up a warning in VS.  I'll review the rest of the library and create a PR and send it to your engineers later this week.
  • EricEric
    None of those functions' implementations return a Task object.  Where are you seeing that version of the code?

    Are these out of order writes only occurring on Android?  I haven't seen these issues in unit testing and .net/uwp apps.
  • FrozenKiwiFrozenKiwi
    Oh yeah, sorry - I copied and pasted in my modified version of the code.  In the original, they don't return Task (which is the problem).

    I see very different threading patterns in UWP vs Android, which is to be expected given the vastly different platform they are running on
  • EricEric
    edited December 2017
    Making every function that internally calls sendCommand return a Task is not an ideal solution.  The top level interface should not care about the implementation details and nor should the user i.e. the API should not be built around the assumption that an implementation requires async functions.  

    I would instead queue the commands for the Android platform to enforce the in-order writes.  The Android SDK does something similar for GATT operations.
  • FrozenKiwiFrozenKiwi
    >> The top level interface should not care about the implementation details and nor should the use

    Async is not an implementation detail.  It is a promise.

    The IBluetoothLeGatt interface explicitly defines async methods.  It is the responsibility of the caller to ensure that this interface is used appropriately.  It does not seem appropriate to require an implementation to know how it is being (mis)used and compensate.  An async method should be awaited if necessary, and ignored if not.  Clearly, these calls should be awaited.

    It is strongly recommended to not call async methods from synchronous methods.  https://msdn.microsoft.com/en-us/magazine/jj991977.aspx.  This bug is one of the reasons for this recommendation.
  • FrozenKiwiFrozenKiwi
    Not to mention any perf gain that comes from not awaiting the calls is almost certainly nix'ed by all the threading overhead.

    > Making every function that internally calls sendCommand return a Task is not an ideal solution.

    An ideal solution would have consistent use of async/await, so my UI doesn't freeze when finalizing the connection :-).
  • EricEric
    Yes they are in that the function now returns a Task now instead of simply being a void function, which changes the meaning of the function and what the user expects it to do.  The top level interfaces defined in the MetaWear namespace are based around the expected behavior when communicating with the board.  Most MetaWear commands do not expect a response, thus are defined to return void to denote that.

    Perhaps a better compromise is to have the MetaWearBoard class account for the Android specific threading issues with its current usage of the IBluetoothLeGatt interface.

    No one was forcing you to synchronously wait for the connect task to finish in your previous implementation's constructor ^^v.
This discussion has been closed.