Template server: some improvements

[mhier]

Some aspects in the example_template are not ideally organised:

• The application name should be overrideable through an optional constructor argument.
• All configuration files (esp. dmap and ConfigReader) should use the application name as a base name
  • <applicationName>.dmap
  • <applicationName>-config.xml
  • also: <applicationName>.xlmap (for multiple devices some suffix would be appended)
• These two changes allow tests to use different configuration files than the example application.
• Test source code should be moved into a subdirectory executable_src. This is important if other source files for the tests are needed (e.g. a dummy backend). This is already de-facto standard in ChimeraTK even if no other sources files are used.
• Do not use a CMakeLists.txt for the tests, because this will place the test executables also into the tests subdirectory. They won't find the config files that way.
• Connect everything to the control system:
  • findTag(".*").connectTo(cs)
  • Remove config.connectTo(cs)
  • Move everything in the config file into a <module name="Configuration">
  • This allows instantiating modules which are automatically connected to the CS
  • It also allows to set any module inputs to constant values through the config file, which is really convenient!
• Publish server version as PVs by default - this is a very good practice and should be done by all servers. Maybe there should be a standard module for this (provided through ApplicationCore itself like e.g. the ConfigReader).
• In defineConnections() the server prints its version together with its name, but the name is hardcoded. This is easily overlooked. Why not use getName() to obtain the application name here?
• Find a way to include the config version in the server version (patch level of the config package) - unclear how to do this since no server is currently doing it.
• In CMakeLists.txt:
  • do not SHOUT (write all commands in lower case)
  • move as much as possible into the project template. By using aux_source_directory() basically all explicit file names can be eliminated (already the case, at least almost). We can take the directory structure for granted, maybe even file names like Server.cc and ApplicationInstance.cc.
  • This also allows to eliminate the CMakeLists.txt in subdirectories, which always make it hard to find problems and has sometimes subtle side-effects.
• Library organisation: Why put only the modules into the library but not the Server.cc etc? This might lead to unnecessary recompilation of stuff for each test. Imagine having a big templated module and many tests... Only the ApplicationInstance.cc must not go into the library.

[mhier]

@ckampm Please have a look at this ticket, maybe you have something to add?

[ckampm]

The implementation ticket #64 for the template server is not finished yet. In that sense, I see these points as additional requirements. I think the points regarding ApplicationCore should all be included. That ticket also is a base for a generic device-to-control system server, #23.

The points about publishing server and config versions should become follow-up tickets of #64.

I partly disagree on the points regarding CMake. While implementing the template to it's current state, I was experimenting with arrangement of the files. I don't consider it final. I personally like having CMakeLists.txt in the subdirectories. I think it reflects more clearly what is arranged where and seems to be somewhat common practice, see e.g. here. Also, the way we use aux_source_directory() is discouraged in the documentation and I agree with the argumentation given there. Some points mentioned, placement of test executables and avoiding recompilation, can also be solved using this structure (I didn't get Qt Creator to set the run directory automatically, but ctest and running the test manually works and the config files are found). I'm not sure if making the test target definition as generic as possible is a proper use case for a cmake module. Tailoring the tests to corner cases of specific dependencies, flags, etc. may be more difficult if it's done by a fixed module.

I think both ways to go have their pros and cons and it comes close to a matter of taste. So we should agree on one solution and use that one. It does not have to be the current way because our CMakeLists.txt anyhow should get refactoring and be moved to modern CMake features and would change in that process.

[mhier]

I partly disagree on the points regarding CMake. While implementing the template to it's current state, I was experimenting with arrangement of the files. I don't consider it final. I personally like having CMakeLists.txt in the subdirectories.

I don't care so much about CMakeLists.txt in subdirectories, but:

• Test executables should be in the root folder of the build directory, to avoid the need for either running tests in a different work directory as the executables are, or to have all config files copied in two places.
• If we have good cmake macros in our project-template, it's pointless to create a CMakeList.txt in a subdirectory with just one line inside.
• It doesn't replace having good macros, because macros are shared across projects. We need to eliminate code duplication for cmake as well!

Also, the way we use aux_source_directory() is discouraged in the documentation and I agree with the argumentation given there.

The arguments are aiming at a different scenario. We have to maintain many similar projects. If we follow that recommendation and name all source files explicitly in our CMakeLists.txt, it greatly reduces the possibilities to use common macros for all projects. It is simply more work to maintain that stuff. Why shall we not let aux_source_directory() do that work for us? And btw: I have never in my live seen a problem from that command which would have not appeared when mentioning the files explicitly. The reasoning is purely academic. From an academic point of view I can agree with it, but it practiseaux_source_directory() is definitively more helpful. It simply reduces the amount of maintenance work!

Some points mentioned, placement of test executables and avoiding recompilation, can also be solved using this structure

Yes, but you have to change the scheme what to but in which library. You will quickly see, that suddenly the subdirectory structure does no longer match the structure of the produced binaries. Either you have to change the subdirectory structure, or the CMakeLists.txt in the subdirectories will become really hard to understand...

I didn't get Qt Creator to set the run directory automatically, but ctest and running the test manually works and the config files are found.

That didn't work for me, at least not when running them naively manually. And I don't want to reconfigure QtCreator everytime for this, this is just unnecessarily annoying (even if of course possible, but why do you want this?).

I think both ways to go have their pros and cons and it comes close to a matter of taste.

Avoiding manual work is not a matter of taste. It's a must for us. We have too many projects.

So we should agree on one solution and use that one.

My only condition is: Reduce manual work as much as possible. This means: config files and tests in the same directory, and no need to put all source file names explicitly into the CMakeLists.txt. Also everything complicated should go into macros, so the CMakeLists.txt becomes only a few lines long and only contains essential information which actually differs for all projects.

It does not have to be the current way because our CMakeLists.txt anyhow should get refactoring and be moved to modern CMake features and would change in that process.

I am perfectly fine with using modern cmake features, but the above conditions are more important. In fact I would not consider something "modern" if it involves more manual work.

[killenb]

I completely agree with @mhier
Having to run cmake again with one common CMakeLists.txt that uses aux_source_directory() and can be used in many projects far outweighs the massive annoyance to keep track of each and every individual source file.

In addition, you have to run cmake manually anyway if you update the config files for testing. This does not go away, even if you list them because CMake does not consider them as source and does not track changes. In fact, this happens far more often than adding or removing a C++ source file.

[mhier]

Besides, we actually moved away from mentioning source files individually to the use of aux_source_directory() a while ago, for exactly those reasons.

[mhier]

(I added one point about the printed name in defineConnection())

[mhier]

now tracked in redmine #8360