Added support for std::vector strings in UrDriver

[PabloDArandaR]

Since it is a functionality allowed by the RTDEClient, seems reasonable to also add it to the UrDriver.

[codecov]

Codecov Report

❌ Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.84%. Comparing base (1d785d9) to head (9c0f108).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
src/ur/ur_driver.cpp	80.00%	2 Missing and 2 partials ⚠️
src/control/reverse_interface.cpp	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #408      +/-   ##
==========================================
- Coverage   42.87%   42.84%   -0.04%     
==========================================
  Files          99       99              
  Lines        8636     8657      +21     
  Branches     1171     1179       +8     
==========================================
+ Hits         3703     3709       +6     
- Misses       4654     4664      +10     
- Partials      279      284       +5     

Flag	Coverage Δ
start_ursim	82.52% <ø> (-2.15%)	⬇️
ur20-latest	40.84% <82.75%> (+0.07%)	⬆️
ur5-3.14.3	40.68% <82.75%> (+0.11%)	⬆️
ur5e-10.7.0	35.89% <79.31%> (+0.20%)	⬆️
ur5e-5.9.4	40.86% <79.31%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[urfeex]

Thank you for the contribution! I also wanted to do that a couple of times already, it just never wasn't important enough. I'll have to think a bit about potential side effects and we would need to add some tests for this.

[PabloDArandaR]

I will think about the tests for starters, specially since with this change we are avoiding the error messages occurring when there is no recipe file defined for the driver (which throws an error with a wrong '' file message).

[urfeex]

I will think about the tests for starters, specially since with this change we are avoiding the error messages occurring when there is no recipe file defined for the driver (which throws an error with a wrong '' file message).

Yes, exactly.

I think, there should be an error raised when

• No output recipe file and no output recipe vector are set
• A specified filename does not exist

A warning should be raised if

• Both, filenames and vectors are given indicating which takes priority (This should be checked for inputs and outputs separately if we want to support mixing as mentioned below).

Do we want to allow e.g. defining the output recipe from a file, but the input recipe from a vector (or vice versa)? My initial answer would be "yes".

Tests should then cover all the different permutations and check for the expected success / error.

---

Edit: The above behavior should also be documented in the Configuration struct, so it is clear to users without looking at the tests or the implementation.

[PabloDArandaR]

improved the logic with your suggestions @urfeex . Only the tests are left. Will try to get them done before the end of this week, but cannot promise it 😅

I made the readRecipe function static public, which design-wise it isn't the most appealing, probably an interface that is in charge of all the system IO (referring to files, not the networking) might be better, but since there aren't many other things that need to be parsed it might be an overkill.

[PabloDArandaR]

@urfeex Okay, I added some tests to the UrDriver to use the new functionality of the input parsing. Since they are not affecting each other, I made only 1 test for each of the following cases since the input/output are handled independently: only recipe files, only recipe vectors, and both at the same time.

[urfeex]

This is definitively a breaking API change, which you also experienced changing all of the examples. May I suggest adding a second constructor accepting the vectors instead of the filenames? Alternatively, add this signature as second constructor and maybe deprecate the existing one.

[PabloDArandaR]

We can do that, but one particular case won't be tested (both the files and the vectors are defined). Since this is only used for testing, I thought about adding a constructor that supports both inputs simultaneously (besides the current constructor). But to keep it consistent, I will add a third constructor that only has the vectors of strings

[urfeex]

I know, I did suggest the deprecation myself, but I think, we don't want to go down that route. Having two constructors, one with recipe files and one with recipe vectors should be sufficient. Then users can pick. Please also leave the examples as is for the scope of this PR.

For testing both being defined we don't need the example wrapper, the UrDriver can be used directly.

[urfeex]

I think we can simplify the logic by always using the vector-based constructor internally. Also note, that an empty / no input recipe is allowed.

You seem to have removed the tests. Adding tests to the UrDriverTest suite should be quite straightforward.

[PabloDArandaR]

Indeed, logic does look cleaner and more correct with your suggestion. I created some test with the UrDriverInit test class type