fbarresi / BeckhoffHttpClient

Unofficial TwinCAT function for HTTP-Client and Json conversion
MIT License
64 stars 9 forks source link

Json Array Isn't Serialized correctly or written to any array in the PLC #6

Closed dhullett08 closed 4 years ago

dhullett08 commented 4 years ago

This happens because when the response is in the form of a JSON array and starts with "[" it is of type JArray not JObject. I created a conditional to check for this and handle accordingly as well as use the base Response tag path to point to the array in the plc then add the [index] in a loop to iterate through all results and write to the plc. There may be a better way to handle this. Just wanted to raise the issue and a possible soultion.

dhullett08 commented 4 years ago
            if (response.Content.StartsWith("["))
            {
                logger.Debug($"Json Array Payload Detected");
                var jsonResponse = JArray.Parse(response.Content);
                if (!string.IsNullOrEmpty(Response))
                {
                    logger.Debug($"Wrinting json response into {Response}...");
                    int index = 0;
                    foreach (JObject j in jsonResponse)
                    {
                        await adsClient.WriteJson(Response + "[" + index + "]", j);
                        index++;
                    }

                }
            }
            else
            {
                var jsonResponse = JObject.Parse(response.Content);
                logger.Debug($"Json Object: {jsonResponse.ToString()} ");
                if (!string.IsNullOrEmpty(Response))
                {
                    logger.Debug($"Wrinting json response into {Response}...");
                    await adsClient.WriteJson(Response, jsonResponse);
                }
            }
fbarresi commented 4 years ago

Hi! If I understand you, your API replies with something like this:

[1, 2, 3]

instead of something like this:

{ "Numbers" : [1, 2, 3] }

Is that right? Since both are valid Json, I guess it should work like TwinCAT.JsonExtension does.

Did you get any error into the logfile? Can you post it here? You can activate the logfile from your installed version putting the file log.config in the folder C:\TwinCAT\Functions\Unofficial\BeckhoffHttpClient\ Can you describe your variables assets?

dhullett08 commented 4 years ago

no the original version wouldnt work with a root json array with no key as in [{"data1": "1,2,3,4"}, {"data2" : "1,2,3,4"}] because a json array is returned with objects inside. The api i was testing with is http://dummy.restapiexample.com/

So the edit i made to a local copy of your code checks for this condition and used JArray instead of JObject which can parse this type of data.

fbarresi commented 4 years ago

Ok, now I understand you! The software expects here an object, but get a list. That throws a JsonReaderException.

Your solution is ok, but it doesn't work recursively.

According to me, this part of the logic should be included into TwinCAT.JsonExtension. After that we can change the TFU001 and make it try parsing an object or, if it fails, an array.

fbarresi commented 4 years ago

Hi! I fixed this issue. Please install and try the new version in your application and let me know. Thank you for starting that and have a great 2020!

dhullett08 commented 4 years ago

It reads successfully now but does not write to the array in the plc correctly. Its writing the same element from the list into every index of the array.

dhullett08 commented 4 years ago

Fixed

    private static async Task WriteArray(this IAdsSymbolicAccess client, string variablePath, JArray array, bool force = false)
    {
        var symbolInfo = (ITcAdsSymbol5)client.ReadSymbolInfo(variablePath);
        var dataType = symbolInfo.DataType;

        if (dataType.Category != DataTypeCategory.Array) throw new InvalidOperationException($"Type of plc variable {variablePath} must be an array");

        var elementCount = array.Count < dataType.Dimensions.ElementCount ? array.Count : dataType.Dimensions.ElementCount;
        for (int i = 0; i < elementCount; i++)
        {
            if (dataType.BaseType.ManagedType != null)
            {
                await WriteAsync(client, variablePath + $"[{i + dataType.Dimensions.LowerBounds.First()}]", array[i]).ConfigureAwait(false);

            }
            else
            {
                await WriteRecursive(client, variablePath + $"[{i + dataType.Dimensions.LowerBounds.First()}]", array[1] as JObject, string.Empty, force).ConfigureAwait(false);
            }
        }
    }

await Write Recursive had array[1] instead of array[i]

Also I had to change to using the WriteJson overload with force to True as the other dosnt call writeArray but only writeJson? Was this intended?

fbarresi commented 4 years ago

Thank you for debugging this! I updated the library and with the fix.

Yes, since JArray and JObject are both valid JSONs there is a new method overload for a JArray now.

dhullett08 commented 4 years ago

Right but shouldn’t the overload for JArray without force parameter also call WriteArray instead of WriteJson since were passing it a JArray?

dhullett08 commented 4 years ago

Nevermind. I see that your calling WriteArray recursively by returning Task WriteJson with the force parameter included to call that overload. Thank you for your work here! This is a fantastic library!