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 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 42.96%. Comparing base (9d81580) to head (3f2b7a1).
⚠️ Report is 3 commits behind head on master.

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #408      +/-   ##
==========================================
+ Coverage   42.94%   42.96%   +0.01%     
==========================================
  Files          99       99              
  Lines        8642     8659      +17     
  Branches     1171     1179       +8     
==========================================
+ Hits         3711     3720       +9     
- Misses       4652     4657       +5     
- Partials      279      282       +3     

Flag	Coverage Δ
start_ursim	81.85% <ø> (-2.82%)	⬇️
ur20-latest	40.95% <96.15%> (+0.21%)	⬆️
ur5-3.14.3	40.71% <96.15%> (+0.10%)	⬆️
ur5e-10.7.0	35.95% <96.15%> (+0.16%)	⬆️
ur5e-5.9.4	40.86% <96.15%> (+0.11%)	⬆️

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]

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

[urfeex]

@PabloDArandaR I hope you don't mind me stepping in on this. From my point of view this looks good as it is right now.

[urfeex]

@PabloDArandaR would you like to change anything on this or is it ready to merge from your side?

[PabloDArandaR]

@urfeex No problem! I just saw some of my changes were also a bit sloppy. I will take a look to the changes and let you know

[PabloDArandaR]

Ready to merge