Add variant support - Githubissues

Hopperpop commented 3 years ago

Feature for setting a variant with an optional parameter -r (--Variant). If a variant isn't provided, TwinCat uses the latest selected variant by default (stored outside the project?).

As ITcSysManager14 is used instead of ITcSysManager10, suggestion are welcome to keep it backwards compatible with older versions.

sagatowski commented 3 years ago

What happens when you try to load a TwinCAT 4020-project (no variant handling) with this?

sagatowski commented 3 years ago

By the way, thanks for this pull request. Looks very exciting. I'll try to review it during the week.

densogiaichned commented 3 years ago

As ITcSysManager14 is used instead of ITcSysManager10, suggestion are welcome to keep it backwards compatible with older versions.

Couldn't we reference TCatSysManagerLib.dll directly and ship it along with TcUnit-Runner, thus not relying on the assembly cache?

Maybe sanity check if version > 4024.0 and variant flag set, pseudo code:

  if (!String.IsNullOrEmpty(Variant) && TcVersion >= 4024.0)
  {
      try
      {
          automationInterface.ITcSysManager.CurrentProjectVariant = Variant;
          log.Info("Variant selected with name: " + Variant);
      }
      catch
      {
          log.Error("Unable to set variant: " + Variant + ". Please provide an existing variant from the project.");
          CleanUpAndExitApplication(Constants.RETURN_INVALID_VARIANT);
      }
  }

sagatowski commented 3 years ago

I wrote this suggestion in the matching issue: One way to handle the limitation that it's only available from 4024 is to be explicit and inform (in documentation + software itself) that the "Variant-Option" is only possible to use from 4024.(0?) and later. In that case we would though have to make sure that:

if anything < 4024.0 is loaded AND the option is provided, the user gets an error message
if a variant is provided, and that variant is not available in the configuration, the user gets an error message

@densogiaichned It seems more or less that is what your pseudo-code is doing.

@densogiaichned I never thought about which version of the TcCatSysManagerLib.dll is actually used! When looking at my installation on my dev-laptop I find several versions:

I guess the version 3.3.0.0 is installed together with the 4024 XAE? It seems the version 3.2.0.0 is what is referenced from the project, which was probably the latest version I happened to have when I started TcUnit-Runner (and that was probably done on a computer with 4022.x).

What happens if we include the 3.3.0.0 DLL, and run TcUnit-Runner on a machine with only the XAE for 4020.x or 4022.x?

sagatowski commented 3 years ago

Linking this to #25, to have an issue for keeping history/track of why a PR was done (so I can use it in the release notes).

Hopperpop commented 3 years ago

The dll version was indeed my biggest concern regarding backwards compatibility. If we ship it with TcRunner we need to check if the license allows it. Or we need to find a way to detect what is installed and disable this feature if 3.3.0.0 isn't found.

Regarding the older XAE versions. I haven't tested it yet with < 4024.0, but with the above psudo-code I don't see any extra benefit unless we display a specific message to inform the user he is using an older version. Either way, 3 things can happen:

The user provides a variant while using an older version.
The user provides a variant while there isn't a variant configured in the project.
The user provides a non-existing/misspelled variant.

In all these cases TcRunner will inform the user that it can't set the variant. If I was the user I would open the project to check if the variant is there. If it's not (because it's not configured/ or not supported), the user knows the problem is not with TcRunner but with the project itself. You can also see it the other way around: How would the user know what variant to provide if he never configured it? In my opinion adding extra sanity check doesn't help that much. Adding it to the documentation is enough in my eyes. (This is of course with the assumption that the new code doesn't brake when using < 4024.0 project. Still needs to be tested.)

sagatowski commented 3 years ago

In all these cases TcRunner will inform the user that it can't set the variant. If I was the user I would open the project to check if the variant is there. If it's not (because it's not configured/ or not supported), the user knows the problem is not with TcRunner but with the project itself. You can also see it the other way around: How would the user know what variant to provide if he never configured it? In my opinion adding extra sanity check doesn't help that much. Adding it to the documentation is enough in my eyes. (This is of course with the assumption that the new code doesn't brake when using < 4024.0 project. Still needs to be tested.)

Makes total sense.

But I guess it would still not work to run the project on a 4020 or 4022-machine if we use ITcSysManager14 as done in the code change, right? I guess the ITcSysManager14 is only available in 3.3.0.0, which was included starting with 4024, no? In that case, running it on a pure 4020 or 4022 host would probably not work?

sagatowski commented 3 years ago

Also as @Beidendorfer noted, what happens if we load a project that has at least 2 variants, but none of them are selected?

Hopperpop commented 3 years ago

Also as @Beidendorfer noted, what happens if we load a project that has at least 2 variants, but none of them are selected?

I have been using the old TcRunner with projects with variants without problems. TwinCat remembers the last used variant and keeps using it. One variant is also always the saved one (see: infosys I assume if it's the first time it selects that one. But I never experience a case where it failed because it wasn't defined.

Yesterday I tried to figure out where it stores the last used variant. I saw that every time you run the new TcRunner it flags the used variant in the project file with some tag, but reverting this and running it again without specifying the variant doesn't seems to have effect (= it still runs the last used variant). It looks like it is stored outside the project. What could make sense is that every target remembers with variant last was used, but I don't have confirmation of that.

Hopperpop commented 3 years ago

I tried running it on a machine with 3.1.4022.4 / 3.1.4022.22 installed, and as expected I got this message because TCatSysManagerLib 3.3.0.0 isn't available:

Task name of the TcUnit task not provided. Assuming only one task in TwinCAT solution
AmsNetId to run TwinCAT/TcUnit is not provided. Assuming TwinCAT/TcUnit will run locally '127.0.0.1.1.1'
A TwinCAT version is not provided. Assuming latest TwinCAT version should be used
Timeout not provided.
Variant not provided.
VISUAL_STUDIO_SOLUTION_PATH found!
The filepath to the visual studio solution file is: "C:\TwinCAT\TcRunnerTest\TcRunnerTest\TcRunnerTest.sln"
2021-05-18 17:59:53 [INFO ] - TcUnit-Runner build: 0.9.3.0
2021-05-18 17:59:53 [INFO ] - In TwinCAT project file, found TwinCAT version 3.1.4022.4
2021-05-18 17:59:53 [INFO ] - In Visual Studio solution file, found visual studio version 12.0
2021-05-18 17:59:53 [INFO ] - Version is pinned: False
2021-05-18 17:59:53 [INFO ] - Trying to load the Visual Studio Development Tools Environment (DTE) version 'VisualStudio.DTE.12.0' ...
2021-05-18 17:59:55 [INFO ] - ...SUCCESSFUL!
2021-05-18 17:59:57 [INFO ] - Using the TwinCAT remote manager to load TwinCAT version '3.1.4022.22'...

Unhandled Exception: System.InvalidCastException: Unable to cast COM object of type 'System.__ComObject' to interface type 'TCatSysManagerLib.ITcSysManager14'. This operation failed because the QueryInterface call on the COM component for the interface with IID '{4304D7CF-68A9-4148-8350-B4091A67C98F}' failed due to the following error: No such interface supported (Exception from HRESULT: 0x80004002 (E_NOINTERFACE)).
   at CallSite.Target(Closure , CallSite , Object )
   at System.Dynamic.UpdateDelegates.UpdateAndExecute1[T0,TRet](CallSite site, T0 arg0)
   at TcUnit.TcUnit_Runner.AutomationInterface..ctor(Project project)
   at TcUnit.TcUnit_Runner.Program.Main(String[] args)
Exit code is -532462766

Does someone knows how we could detect and change between available dll's?

Beidendorfer commented 3 years ago

Hello I like the variants manager. I don't have a TC system here on side right now. Maybe it helps #15 ITcRemoteManager remoteManager.Versions. In TcRunner we check the last installed version.

Do you use the Variant Manager for different I/O´s or also for PLC Code? I ask because TcRunner refers to the unit test in the plc code. If the majority takes the variants only in the IO's. Could the classic variant to disable the IO be the better one for TcRunner?

I just want to tell you one more experience with the Variant Manager. I use many PLC programs with variants and Git. And I remember when I change the branch or repo and want to activate the configuration, there is a TcSystem Error 1000. This is related to the fact that no variant is selected. After selecting the variant everything runs without error. This indicates to me that the variant config is outside the project.

I plan to test different programs with different versions on my test server. Here I could imagine that there are problems if the last saved variant is not in the project.

If you want me to test something else let me know.

maybe an idee: If the TC Version is >= 4024.0 and the Option variant in TC Runner is != Null. Then we can call a Methode in the Automation Interface withITcSysManager 14to select the correct version instead of using the ITcSysManager 14 in the whole Project . The Reset of TC Unit is using then ITcSysManager 10

densogiaichned commented 3 years ago

@Hopperpop Just curious, did you copy the TCatSysManagerLib 3.3.0.0 DLL and update the project reference to point to this local dll instead of GAC, as i suggested before? IMHO this should work...

sagatowski commented 3 years ago

@densogiaichned The problem is that we would have to investigate the licensing issues. I think the solution that @Beidendorfer has suggested to use different versions of the ITcSysManager depending on which version of TwinCAT we are using sounds like a very good idea. Basically going with ITcSysManager10 as a starting point, and then "upping" it to ITcSysManager14 if the used project is > 4024.0.

Hopperpop commented 3 years ago

I made a version that first check the used twincat version and only uses ITcSysManager14 for setting the variant. They worked on both my machine (with and without dll 3.3.0.0).

(On a side note: I haven't used C# before, I'm just learning on the fly. I recommend that someone reviews the code for rookie mistakes. For example: Do I need to close any references?)

sagatowski commented 3 years ago

@Hopperpop Thank you for your commits! I really like the update you've done to the BAT-script. Looks much cleaner now!

I've reviewed your code and have only a few questions: Is it possible to use the class AutomationInterface for the sysmanager instead? This class already takes care of the SysManager right now, and I don't think it makes sense to have an instance of the sysmanager both in the Program.cs and in the AutomationInterface.cs.

Also, I saw that you changed the assembly from 3.2.0.0 to 3.3.0.0. I guess this means that a machine with 4024 is required to build the project now? Will it still run on a machine that only has 4020 or 4022 installed? (even though the assembly says 3.3.0.0)?

densogiaichned commented 3 years ago

I double checked @Hopperpop solution on a freshly installed 3.1.4020.10 and it works.

Is it possible to use the class AutomationInterface for the sysmanager instead? This class already takes care of the SysManager right now, and I don't think it makes sense to have an instance of the sysmanager both in the Program.cs and in the AutomationInterface.cs.

This does not work, because ITcSysManager14 is only allowed to be referenced for VariantManager:

  /* Select variant if provided as parameter
   */
  if (!String.IsNullOrEmpty(Variant))
  {
      if (Utilities.CompareVersionStrings("3.1.4024.0", vsInstance.GetLoadedTcVersion()))
      {
          try
          {
              //Using newer version of ITcSysManager to be able to set variant
              ITcSysManager14 sysManager = (ITcSysManager14)vsInstance.GetProject().Object;
              sysManager.CurrentProjectVariant = Variant;
              log.Info("Variant selected with name: " + Variant);
          }
          catch
          {
              log.Error("Unable to set variant: " + Variant + ". Please provide an existing variant from the project.");
              CleanUpAndExitApplication(Constants.RETURN_FAILED_TO_SET_VARIANT);
          }
      }
      else
      {
          log.Error("Variants are only supported from Tc3.1.4024.0 or higher.");
          CleanUpAndExitApplication(Constants.RETURN_FAILED_TO_SET_VARIANT);
      }
  }

If we would reference the newer version in class AutomationInterface, the System.InvalidCastException will occure, because the older dll does't know about ITcSysManager14. This is basically a trade-off between backwards-compatibility and only having to have one TcUnit-Runner.exe (not compiling for diffrent versions).

Some options one could think of:

Cast the AutomationInterface.ITcSysManager property

//Using newer version of ITcSysManager to be able to set variant
ITcSysManager14 sysManager14 = (ITcSysManager14)automationInterface.ITcSysManager;
sysManager14.CurrentProjectVariant = Variant;
log.Info("Variant selected with name: " + Variant);

For the sake of completeness, but we should not consider this option:
Change AutomationInterface to a generic class, use of dynamic keyword and early decide which TwinCAT version is loaded AutomationInterface.cs

class AutomationInterface<TITcSysMgr> where TITcSysMgr : ITcSysManager10
{
    private TITcSysMgr sysManager;
    private ITcConfigManager configManager = null;
 . . .

Program.cs

dynamic automationInterface;

if (Utilities.CompareVersionStrings("3.1.4024.0", vsInstance.GetLoadedTcVersion()))
    automationInterface = new AutomationInterface<ITcSysManager14>(vsInstance.GetProject());
else
    automationInterface = new AutomationInterface<ITcSysManager10>(vsInstance.GetProject());

Hopperpop commented 3 years ago

Also, I saw that you changed the assembly from 3.2.0.0 to 3.3.0.0. I guess this means that a machine with 4024 is required to build the project now? Will it still run on a machine that only has 4020 or 4022 installed? (even though the assembly says 3.3.0.0)?

I was also able to run it on a system with only 4022 installed. I was thinking if it would be possible to use something like pragma's to detect the available libraries on the building machine and disable the variant support if the new version isn't there. Only downside is that there will be 2 release versions who will be hard to distinguished from each other, but you will be able to build it on all systems.

@densogiaichned point seems valid. But I don't see the big benefit between what we have now and doing the cast trick. As both create a new sysManager object. It's just doing it in a different way. Or am I wrong?

densogiaichned commented 3 years ago

@Hopperpop you are right, doesn't make any difference. I was addressing @sagatowski point/question

Is it possible to use the class AutomationInterface for the sysmanager instead? This class already takes care of the SysManager right now, and I don't think it makes sense to have an instance of the sysmanager both in the Program.cs and in the AutomationInterface.cs.

sagatowski commented 2 years ago

Also, I saw that you changed the assembly from 3.2.0.0 to 3.3.0.0. I guess this means that a machine with 4024 is required to build the project now? Will it still run on a machine that only has 4020 or 4022 installed? (even though the assembly says 3.3.0.0)?

I was also able to run it on a system with only 4022 installed. I was thinking if it would be possible to use something like pragma's to detect the available libraries on the building machine and disable the variant support if the new version isn't there. Only downside is that there will be 2 release versions who will be hard to distinguished from each other, but you will be able to build it on all systems.

@Hopperpop Hi. Let's wake this PR up again, as more and more people are asking for this functionality. So, it is possible to run TcUnit-Runner on a 4020/4022-machine, even though it's compiled with 3.3.0.0 of the library? This seems to contradict it, no?

The usecase is simply that I want to be able to make sure that anyone that is running a version of TwinCAT that does not have variant management (so all 4020 and 4022), should still be able to do it using TcUnit-Runner. I think I need to spin up some virtual machines with old versions of TwinCAT to verify that this PR would not break anything.

sagatowski commented 6 months ago

This project is archived. Replacement for it is in the works.

tcunit / TcUnit-Runner

Add variant support #24