Tag group API

[kyle-github]

I wanted to break these out from #66 as that is getting a bit long!

Do you need to have constructors for tags within the TagGroup class? This seems like it may unnecessarily couple Tags and TagGroups. Perhaps I am missing something subtle here. At least in my mental model of this, Tags would be created independently and then grouped after they were created. My mental model also allows a Tag instance to be in more than one group at a time. It seems like that is possible here too.

On the read/write methods, does the break out of two kinds of async help? Perhaps I am just not seeing common use cases here. At least in my own code (the plural of anecdote is not data!), I either use blocking with a timeout or I do all the timeout management myself. The difference seems to be in the existence of the timeout parameter. What happens if you provide a timeout parameter? Does it automatically throw an exception on timeout? On the blocking method, what happens on timeout?

I am not used to C# async code, so if this model is a common idiom, then let me know and I'll shut up and go away :-)

[timyhac]

Alrighty in that case we'll close the other issue.

[timyhac]

Adding constructors to the TagGroup isn't required at all - in fact its probably even more verbose than something like tagGroup.Add(new Tag()). Hmmmm yes it was just because it was more terse having a TagCreate function where you can provide a few details about the tag - but we could just have them as optional parameters in the constructor instead - which would override any of the same details found in the attribute group.

With the additional async method (with the timeout), we thought that it would still be useful to add a timeout to the non-blocking method. This would be useful as you'd often want it to be non-blocking (so UIs can remain responsive) but still timeout on network connection issues.

The API for HttpClient instead has an optional Timeout property that applies to all calls for a given HttpClient , as opposed to applying it per call. Could make sense? @jkoplo One benefit of that is that it could be provided in the constructor, and the default behaviour for the constructor could be a blocking call with the provided timeout - but if they chose to delay it they could ask for that too.

var myDint = new Tag<PLC,CLR>(attributeGroup, "MY_DINT", 5000, initNow: false);
await myDint.InitializeAsync(); // non-blocking, and times out after 5000ms

versus

var myDint = new Tag<PLC,CLR>(attributeGroup, "MY_DINT", 5000);   // blocks and times out after 5000ms

[timyhac]

Following on from the comment about Tag creation from above, I've updated the group branch, the new API is consumed as follows:

var myPlcA = new AttributeGroup()
{
    Gateway = "192.168.0.10",
    Path = "1,0",
    PlcType = PlcType.ControlLogix,
    Protocol = Protocol.ab_eip
};

var myPlcB = new AttributeGroup()
{
    Gateway = "192.168.0.11",
    Path = "1,0",
    PlcType = PlcType.ControlLogix,
    Protocol = Protocol.ab_eip
};

var dint1a = new Tag<DintMarshaller, int>(myPlcA, "MY_DINT_1D[0]");
var dint2a = new Tag<DintMarshaller, int>(myPlcA, "MY_DINT_1D[1]");
var dint1b = new Tag<DintMarshaller, int>(myPlcB, "MY_DINT_1D[2]");
var dint2b = new Tag<DintMarshaller, int>(myPlcB, "MY_DINT_1D[3]");

var tagGroup = new TagGroup()
{
    dint1a, dint2a, dint1b, dint2b
};

var timeout = 1000;
tagGroup.InitializeAll(timeout);
tagGroup.ReadAll(timeout);

var value = dint2b.Value[0];

Console.WriteLine(value);

[AndreasMartinGB]

Do you already know when you plan to merge the Group branch into the master (and maybe also the other branches)? For my project it would be very helpful.

Thx in advance

[timyhac]

@AndreasMartinGB - no plans at the moment. It is a reasonably big change from an API perspective, and motivation to integrate it working has dropped somewhat (from me).

Just out of interest - what is your use case here, what interests you about the group branch?

[AndreasMartinGB]

I want to be able to configure the tags to read and write. My idea here was to add the specific base type mapper to the tag group item based on the configuration. From code perspective it would feel cleaner if this would be supported directly but as there is no plan for merging this is also no big deal to do it by my own. Thx for your answer.