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

[Bug Report] PnP APIs. TryGetValue fails with ACK properties #2065

Closed
rido-min opened this issue Jul 7, 2021 · 3 comments
Closed

[Bug Report] PnP APIs. TryGetValue fails with ACK properties #2065

rido-min opened this issue Jul 7, 2021 · 3 comments
Assignees
Labels
bug Something isn't working. fix-checked-in Fix checked into main or preview, but not yet released. IoTSDK Tracks all IoT SDK issues across the board

Comments

@rido-min
Copy link
Member

rido-min commented Jul 7, 2021

Reported properties can use the ACK format (ac, av, ad, value).

I expected CloudProperty.TryGetValue knows about that format and tries to deserialize the value if present.

Repro code

using Microsoft.Azure.Devices.Client;
using Microsoft.Azure.Devices.Shared;
using System;
using System.Threading.Tasks;

namespace Rido.Samples
{
    
    class Program
    {
        static async Task Main(string[] args)
        {
            var mid = "dtmi:rido:simulator;1";
            var CS = Environment.GetEnvironmentVariable("CS");
            var dc = DeviceClient.CreateFromConnectionString(CS, TransportType.Mqtt, new ClientOptions { ModelId = mid });

            bool SendDataEnabled = true;

            //device reports the default value using the ack format
            IWritablePropertyResponse ack = dc.PayloadConvention.PayloadSerializer
                         .CreateWritablePropertyResponse(
                             SendDataEnabled, CommonClientResponseCodes.OK,1);

            var propertiesToBeUpdated = new ClientPropertyCollection();
            propertiesToBeUpdated.AddRootProperty("SendDataEnabled", ack);
            await dc.UpdateClientPropertiesAsync(propertiesToBeUpdated);

            var props = await dc.GetClientPropertiesAsync();
            // TryGetValue fails because the property is not bool
            if (props.TryGetValue<bool>("SendDataEnabled", out SendDataEnabled))
            {
                Console.WriteLine("Found reported SendDataEnabled: " + SendDataEnabled);
            } 
            else
            {
                Console.WriteLine("reported SendDataEnabled not found");
            }

            Console.WriteLine("Enter to Exit");
            Console.ReadLine();
        }
    }
}
@rido-min rido-min added the bug Something isn't working. label Jul 7, 2021
@github-actions github-actions bot added the IoTSDK Tracks all IoT SDK issues across the board label Jul 7, 2021
@jamdavi jamdavi self-assigned this Jul 12, 2021
@abhipsaMisra
Copy link
Member

@rido-min I think there is a mismatch in the expectations of calling TryGetValue on a writable property.
(1) To retrieve the value of service requested writable property update - props.TryGetValue<bool>("SendDataEnabled", out SendDataEnabled)
(2) To retrieve the value of device reported writable property value - props.TryGetValue<NewtonsoftJsonWritablePropertyResponse>("SendDataEnabled", out SendDataEnabled)

For (2), the expectation is that you would get the complete writable property response that is associated with that property (value, ac, av, ad (if present)). You can call .Value on it to fetch the actual value.

This does necessitate the knowledge of NewtonsoftJsonWritablePropertyResponse though, so I agree that it isn't the most intuitive API.
I am going to link this issue to #2097, since they touch the same topic of making a writable property response more discoverable.

@abhipsaMisra
Copy link
Member

We've now added support for TryGetValue with the following features:

For writable property update responses reported from the client, you should be able to retrieve the property as:

  • generic IWritablePropertyResponse
  • your implemented IWritablePropertyResponse
  • the .Value part of the IWritablePropertyResponse

@abhipsaMisra abhipsaMisra added the fix-checked-in Fix checked into main or preview, but not yet released. label Aug 31, 2021
@abhipsaMisra
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. fix-checked-in Fix checked into main or preview, but not yet released. IoTSDK Tracks all IoT SDK issues across the board
Projects
None yet
Development

No branches or pull requests

3 participants