[Issue 79] Fix permissions tutorial

[ross-desmond]

Summary

• SROS2 assumed services were included in the policy yaml file
• Format errors in the sample_policy prevented yaml parse

---

SROS2 is moving to xml, if a fix is required in the yaml version, here it is.

Resolves #79

[ross-desmond]

@ruffsl Should this change be required prior to pushing your xml changes we can use this, otherwise I will close this PR.

[mikaelarguedas]

Thanks @ross-desmond !

In order to get the tutorial working we would also need:

• update the file URL
• add all the "default" topics and services to the policy file (as before they were added automatically by sros2 but they now need to be explicitly specified by the user)

As #72 seems pretty close to being merged I agree that it may not be worth the effort updating the to-be-deprecated yaml format 👍

[jacobperron]

In order to get the tutorial working we would also need:

• update the file URL
• add all the "default" topics and services to the policy file (as before they were added automatically by sros2 but they now need to be explicitly specified by the user)

I think it is worth doing this as part of this PR to incorporate as part of the next Crystal patch release.

@ross-desmond @mikaelarguedas What do you think?

[mikaelarguedas]

The conversation from #72 (comment) makes it sound like it's targeted to be released in the next Crystal patch release

[jacobperron]

The conversation from #72 (comment) makes it sound like it's targeted to be released in the next Crystal patch release

I may be mistaken, but I don't think it is explicitly targeted for Crystal, but could be considered. My reservation regarding adding #72 to Crystal is that security will break for users of the previous YAML format. It seems like a significant enough change that releasing it into Dashing would be more appropriate.

[ross-desmond]

In order to get the tutorial working we would also need:

* update the file URL

* add all the "default" topics and services to the policy file (as before they were added automatically by sros2 but they now need to be explicitly specified by the user)

@mikaelarguedas the "default" topics are still automatically added by sros2 if they are not in the policy file. A dictionary de-duplicates these if they are are in the policy file :). This means that all that is left to do is update the file URL.

@jacobperron if that is the case I will go ahead and update the file URL and LGTM

[jacobperron]

@ross-desmond Since #72 has been merged. I've updated this PR to point to crystal.

[mikaelarguedas]

mikaelarguedas the "default" topics are still automatically added by sros2 if they are not in the policy file. A dictionary de-duplicates these if they are are in the policy file :). This means that all that is left to do is update the file URL.

Right I missed that with the services issue fixed it will add the topics for the parameter services 👍 thanks for pointing it out.
I confirm that just updating the URL on top of this change will make the tutorial work 💯

---

I may be mistaken, but I don't think it is explicitly targeted for Crystal, but could be considered. My reservation regarding adding #72 to Crystal is that security will break for users of the previous YAML format. It seems like a significant enough change that releasing it into Dashing would be more appropriate.

TBH I did not follow the PR closely, some comments made me think this was aiming for the patch release: ros2/ros2#647 (comment) and #72 (comment) but I didnt look in details.

I understand the aim for avoiding making breaking changes within a distro 👍.

As a user, I don't have a strong preference as I'll have to update my policy files in both cases:

• if the xml doesnt make it into Crystal I'll update my files to the new yaml format that changed in the first patch release.
  • pros: the changes should be small compared to using a completely different xml based syntax
  • cons:
    • need to update the files again when Dashing is released
    • The ROS 2 tutorials will need to point to the crystal branch of sros2 for users to be able to follow the tutorial
• if it does make it I'll update directly to the new xml syntax.

Maybe @nuclearsandwich @mjcarroll @ruffsl have some preference on the way to go

[ruffsl]

Maybe @nuclearsandwich @mjcarroll @ruffsl have some preference on the way to go

I'm in favor of backporting the xml features into crystal, given the yaml defaults are missing support for required topics such as rosout, a breaking issue. Additionally, in contrast to xml format, the yaml format is still missing support for actions and examples on using services. That said, as the OP of the new composable profile formats, I'm may be bias on the topic.

I've been keeping an eye out for early adopters of the sros features, but have only found Apex using it for performance_test with security enabled, and fastrtps folks when debugging interoperability.

https://discourse.ros.org/t/ros2-security-performance-tests/6718
https://github.com/ApexAI/performance_test
eProsima/Fast-DDS#250

[mjcarroll]

I'm in favor of backporting the new XML features, because as @mikaelarguedas said, there are likely going to be profile updates that have to happen either way. I've been slow getting @ruffsl's PR in, but we were going to attempt to get it in as early as the first Crystal patch release.

[ross-desmond]

@ruffsl I'm happy with either, the sooner the XML features get in, the sooner we can add more features to that code base.

[jacobperron]

• if the xml doesnt make it into Crystal I'll update my files to the new yaml format that changed in the first patch release.

Good point, I overlooked that this already happened.

---

Sounds like there is consensus in backporting the XML features 👍

[jacobperron]

Replaced by #93