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

SetDesiredPropertyUpdateCallback() is not working. #45

Closed
gmileka opened this issue Jan 30, 2017 · 6 comments
Closed

SetDesiredPropertyUpdateCallback() is not working. #45

gmileka opened this issue Jan 30, 2017 · 6 comments
Assignees
Labels
bug Something isn't working.

Comments

@gmileka
Copy link
Contributor

gmileka commented Jan 30, 2017

Using master from https://github.com/Azure/azure-iot-sdk-csharp.git, the callback function is not getting called when the Device Twin desired properties change.

Here's the WPF application code I have tested with:

    public partial class MainWindow : Window
{
private const string DeviceConnectionString = "";
private DeviceClient _deviceClient;

    public MainWindow()
    {
        InitializeComponent();
    }

    private void OnTest(object sender, RoutedEventArgs e)
    {
        _deviceClient = DeviceClient.CreateFromConnectionString(DeviceConnectionString, TransportType.Mqtt);
        _deviceClient.SetDesiredPropertyUpdateCallback(OnDesiredPropertyUpdate, this);
        _deviceClient.SetMethodHandlerAsync("MyMethodAsync", MyMethodAsync, null);

    }

    public Task OnDesiredPropertyUpdate(TwinCollection desiredProperties, object userContext)
    {
        Debug.WriteLine("Property!");
        return null;
    }

    Task<MethodResponse> MyMethodAsync(MethodRequest methodRequest, object userContext)
    {
        Debug.WriteLine("Method!");
        return null;
    }
}
@gmileka
Copy link
Contributor Author

gmileka commented Jan 30, 2017

Based on investigation, the code creates the first chain of inner handlers correctly and sets the callback. However, within the call of SetDesiredPropretyUpdateCallback(), the code resets part of the chain without setting the callback again - so it gets lost.
We did a work around where we checked if the inner handler is present before creating a new one. This unblocked us.

@tameraw tameraw added the bug Something isn't working. label Jan 31, 2017
@gmileka
Copy link
Contributor Author

gmileka commented Feb 1, 2017

Jasmine pointed me to the sample, and the problem is that I’m missing the explicit call to OpenAsync()

            Client = DeviceClient.CreateFromConnectionString(DeviceConnectionString, TransportType.Mqtt);
            Client.OpenAsync().Wait(); // <-- this is missing in my repro.
            Client.SetDesiredPropertyUpdateCallback(OnDesiredPropertyChanged, null).Wait();

Once I added this, things started to work.

This makes sense, because when I debugged SetDesiredPropertyUpdateCallback(), it ended-up calling OpenAsync(), and during that call the callback was lost. In other words, the code path for OpenAsync() does not expect the callbacks to be set, and overwrites them.

So, if SetDesiredPropertyUpdateCallback() is expected to handle the case where OpenAsync() hasn't been called yet - we might still have a bug.

@jasmineymlo
Copy link
Contributor

The is a valid bug and SetDesiredPropertyUpdateCallback() is expected to handle the case where OpenAsync() hasn't been called. Fix is on the way. Thanks.

@jasmineymlo
Copy link
Contributor

commit: ccd8bc9

@olavt
Copy link

olavt commented Feb 9, 2017

I'm trying to run the sample (https://github.com/Azure/azure-iot-sdk-csharp/blob/master/device/samples/DeviceClientTwinMqttSample/Program.cs), but it crashes on:

Client.SetDesiredPropertyUpdateCallback(OnDesiredPropertyChanged, null).Wait();

Connecting to hub

Error in sample: System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.Azure.Devices.Client.Transport.TransportHandlerFactory.Create(IPipelineContext context)
at Microsoft.Azure.Devices.Client.DeviceClient.<>c__DisplayClass24_0.b__4(IPipelineContext ctx)
at Microsoft.Azure.Devices.Client.Transport.DefaultDelegatingHandler.EnsureInnerHandlerInitialized()
at Microsoft.Azure.Devices.Client.Transport.DefaultDelegatingHandler.get_InnerHandler()
at Microsoft.Azure.Devices.Client.Transport.DefaultDelegatingHandler.set_TwinUpdateHandler(TwinUpdateCallback value)
at Microsoft.Azure.Devices.Client.Transport.DefaultDelegatingHandler.set_TwinUpdateHandler(TwinUpdateCallback value)
at Microsoft.Azure.Devices.Client.Transport.DefaultDelegatingHandler.set_TwinUpdateHandler(TwinUpdateCallback value)
at Microsoft.Azure.Devices.Client.Transport.DefaultDelegatingHandler.set_TwinUpdateHandler(TwinUpdateCallback value)
at Microsoft.Azure.Devices.Client.DeviceClient.<>c__DisplayClass60_0.<b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Azure.Devices.Client.TaskHelpers.d__17.MoveNext()

The only modification I did to the code was to set the connection string for my device on the form:

"HostName=<iothub_host_name>;DeviceId=<device_id>;SharedAccessKey=<device_key>"

using my real connection string for the device of course.

I'm using v1.2.2 of the Microsoft.Azure.Devices.Client

@tameraw
Copy link
Contributor

tameraw commented Feb 13, 2017

Fix provided in 1.2.3. Please let us know if still having issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working.
Projects
None yet
Development

No branches or pull requests

4 participants