titanium-as / TitaniumAS.Opc.Client

Open source .NET client library for OPC DA
MIT License
193 stars 94 forks source link

Debug Idea: Return Item with correct ItemId if adding item to group fails #19

Closed ghost closed 6 years ago

ghost commented 6 years ago

Hey,

just an idea/suggestion here - I was adding a lot of OPC items to group and one of them had an error (typo in opc tag) causing the entire group to fail. It took me a while to find it since when the item is added to group the result.Error is set but the item itself is not returned so it's not possible to see which Item caused the error (unless you breakpoint in Titanium code).

I fixed this with a workaround where I return a dummy item with the correct Itemid when result.Error is not null - See my modified CreateItemResults function:

        private OpcDaItemResult[] CreateItemResults(IList<OpcDaItemDefinition> itemDefinitions, OPCITEMDEF[] pItemArray,
            OPCITEMRESULT[] opcDaItemResults, HRESULT[] ppErrors, bool setGroup)
        {
            var results = new OpcDaItemResult[pItemArray.Length];
            for (int index = 0; index < opcDaItemResults.Length; index++)
            {
                OPCITEMRESULT opcItemResult = opcDaItemResults[index];
                OpcDaItemDefinition opcItemDefinition = itemDefinitions[index];
                HRESULT error = ppErrors[index];

                if (error.Succeeded)
                {
                    var item = new OpcDaItem(opcItemDefinition, opcItemResult, setGroup ? this : null)
                    {
                        UserData = opcItemDefinition.UserData
                    };
                    results[index] = new OpcDaItemResult(item, error);
                }
                else
                {
                    // JT we still want the item even if it failed to add (for Debug)
                    var item = new OpcDaItem(opcItemDefinition, opcItemResult, setGroup ? this : null)
                    {
                        UserData = opcItemDefinition.UserData
                    };
                    results[index] = new OpcDaItemResult(item, error);
                }
            }
            return results;
        }

This allows you to print out or handle the Item(s) that caused the issue, i.e:

                OpcDaItemResult[] results = group.AddItems(definitions);

                // Handle adding results.
                foreach (OpcDaItemResult result in results)
                {
                    if (result.Error.Failed)
                    {
                        Log.Error(string.Format("CVGroup error adding items: {0} - {1}", result.Error, result.Item));
                    }
                }

There is probably a better/more elegant way of doing this but just an idea implementation here. This can help a lot with debugging incorrect Opc handles in my opinion.

Keep up the great work!

alexey-titov commented 6 years ago

Thanks for your proposal. The library is quite low-level and uses arrays where it is possible for performance, so we propose to iterate with for(...) instead of foreach. So you can get the name from "definitions".

              OpcDaItemResult[] results = group.AddItems(definitions);

                // Handle adding results.
                for (int i=0; i<results.Count; i++)
                {
                    OpcDaItemResult result = results[i]; 
                    if (result.Error.Failed)
                    {
                        Log.Error(string.Format("CVGroup error adding items: {0} - {1}", result.Error, definitions[i].ItemId));
                    }
                }

Btw we have implemented your idea in our framework based on the library.