UR20 description and meshes #657

urrsk · 2023-09-07T10:58:52Z

The UR20 meshes are added under Universal Robots A/S’ Terms and Conditions for Use of Graphical Documentation

The UR20 meshes are added under Universal Robots A/S' Terms and Conditions for Use of Graphical Documentation

fmessmer · 2023-09-25T09:53:37Z

thanks for the URDF and meshes 🎉
they work great for us....could we get this merged - and released to Noetic - some time soon?

fmauch

Maybe we also want to add a README explaining the licensing situation with the UR20 meshes as we did it for the ROS 2 description?

@gavanderhoorn I feel like this would be a sensible point to create a noetic-devel branch. Technically, this would probably be possible to build and use on ROS noetic, as well, but since melodic isn't actively maintained anymore I feel like adding new features to a melodic-devel branch seems wrong.

ur20_moveit_config/CMakeLists.txt

ur_description/package.xml

fmauch

With the cmake version increase of the UR16e being the only thing not necessarily belonging to this PR I woud be fine merging this. We'll just have to decide on the target branch.

RobertWilbrandt · 2023-10-09T10:53:41Z

I agree with merging this to noetic-devel. As there are no further melodic binary releases, people have to build from sources anyway to get this support and using noetic sets the right expectations about maintenance and robustness.

fmauch · 2023-10-09T10:55:49Z

Alright, I'll merge this on Friday then and start releasing from a noetic branch.

gavanderhoorn

First round of comments.

ur20_moveit_config/CHANGELOG.rst

ur20_moveit_config/launch/planning_context.launch

ur_description/meshes/ur20/LICENSE.txt

ur_description/package.xml

ur_kinematics/CMakeLists.txt

ur_kinematics/ur_moveit_plugins.xml

ur_description/package.xml

Code change suggestions Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de> Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>

Add additional UR20 meshes licens comment Remove Changelog for ur20_moveit

README.md

Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>

urrsk · 2023-10-16T14:25:27Z

Please squash the commits when it is ready for getting merged.

README.md

Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>

README.md

fmauch · 2023-10-30T09:45:02Z

@gavanderhoorn Could you give this another review please?

fmessmer · 2023-11-06T10:14:01Z

Alright, I'll merge this on Friday then and start releasing from a noetic branch.

what's blocking this PR from merging?

fmauch · 2023-11-06T10:24:18Z

what's blocking this PR from merging?

A review requesting changes (or a re-review to be more precise, since these comments have been addressed).

urrsk · 2023-11-14T12:36:35Z

@gavanderhoorn it would valuable if you could fine the time in the near future to review this PR again.
I believe requested changes have been addressed. Otherwise please let me know.

gavanderhoorn

apologies for my delayed reply.

I've left a couple in-line comments, which I expect are easily resolved.

If desired, I can review again tomorrow, or early next week.

README.md

ur20_moveit_config/config/joint_limits.yaml

gavanderhoorn · 2023-11-23T20:45:47Z

ur_description/meshes/ur20/LICENSE.txt

👍 to storing the license file in the repository. It's essentially a contract, and clients should have a copy of that contract if they are going to be bound by it.

gavanderhoorn · 2023-11-23T20:47:22Z

README.md

 [![License](https://img.shields.io/badge/License-BSD%203--Clause-blue.svg)](https://opensource.org/licenses/BSD-3-Clause)

+[![License](https://img.shields.io/badge/License-Universal%20Robots%20A/S%E2%80%99%20Terms%20and%20Conditions%20for%20Use%20of%20Graphical%20Documentation-blue.svg)](https://www.universal-robots.com/legal/terms-and-conditions/terms_and_conditions_for_use_of_graphical_documentation.txt)


Should we instead make this point to the ur_description/meshes/ur20/LICENSE.txt file? The online version could be changed at any time. The files in this repository would fall under the version of the license as it is right now, in this PR. It would make sense to me to make this badge link to the version the files would fall under.

From what I understood, the license is structured such that the latest version always is relevant (see point 1.4)

... If you download a software package that includes the Graphical Documentation, your use of the Graphical Documentation will be subject to and governed by the latest version of the T&Cs, which can be found here: ...

Given that clause I think it does make sense to link to the online version.

I'm not sure that's legal. There would be no change notification to current users, UR would be able to silently, unilaterally change the complete license without anyone being aware of it, and with all licenses I've worked with (and in the jurisdictions I've had to work with licenses), once you have a copy of the artefacts under that license, that copy and/or version will always fall under the version of the license as it was when you agreed to it.

I have seen licenses where there are provisions for updates made by the licensor, but those always include some sort of notification period, allowing current licensees to terminate their use of whatever is under the existing license if they feel they can't, or don't want to, agree to the new version.

Additionally, there is already a copy of the current version in the repository -- or at least there will be, after merging this PR.

It's best practice to include a copy of the license in all cases, even with OSS licenses. We don't do that currently in this repository, but that's an omission and should be fixed (in a later PR).

The same section 1.4 also states:

However, you may only make the Graphical Documentation or any whole or partial copies thereof available to the public via download if it is part of a downloadable software package that includes a copy of the T&Cs.

which is what this PR will result in: a copy will be included.

So it doesn't seem strange to me to then point to that copy, and not the online version.

That seems like it would just lead to confusing situations.

You have a point. Though the motivation was to remove the burden from the repo owner to ensure that the license is up to date.

gavanderhoorn · 2023-11-23T20:52:37Z

I've approved the changes to the previous review so as to not block this.

I've left new comments as comments, so they don't block.

Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>

Added clarifying text reading the UR license to the Readme

fmessmer · 2023-12-10T13:05:36Z

still eagerly waiting for this to get merged and released 🙄

fmauch · 2023-12-18T09:50:07Z

As discussed and requested by UR, I'll leave the license badge point to the online version and merge things as they currently are.

UR20 description and meshes

174e5bb

The UR20 meshes are added under Universal Robots A/S' Terms and Conditions for Use of Graphical Documentation

urrsk force-pushed the ur20 branch from 468e3e1 to 174e5bb Compare September 11, 2023 09:22

fmessmer mentioned this pull request Sep 13, 2023

Add UR20 support #605

Closed

fmauch reviewed Sep 25, 2023

View reviewed changes

ur20_moveit_config/CMakeLists.txt Outdated Show resolved Hide resolved

ur_description/package.xml Show resolved Hide resolved

fmauch mentioned this pull request Oct 5, 2023

Added support for UR20 UniversalRobots/Universal_Robots_ROS_Driver#659

Merged

fmauch approved these changes Oct 9, 2023

View reviewed changes

fmauch changed the base branch from melodic-devel to noetic-devel October 9, 2023 06:55

gavanderhoorn requested changes Oct 10, 2023

View reviewed changes

fmauch mentioned this pull request Oct 13, 2023

Add license comment to package.xml UniversalRobots/Universal_Robots_ROS2_Description#107

Merged

urrsk and others added 3 commits October 13, 2023 08:56

Apply suggestions from code review

7c7aef6

Code change suggestions Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de> Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>

Implement suggestion from code review

a97be91

Add additional UR20 meshes licens comment Remove Changelog for ur20_moveit

Update Readme file with UR20 license comment

fe6bb6f

urrsk force-pushed the ur20 branch from 5b29d14 to fe6bb6f Compare October 13, 2023 08:59

Add contact email regarding UR20 license

b94bebe

fmauch reviewed Oct 13, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

fmauch requested a review from gavanderhoorn October 13, 2023 12:51

Add README suggestion regarding license

3412dfe

Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>

fmauch reviewed Oct 17, 2023

View reviewed changes

README.md Show resolved Hide resolved

fmauch mentioned this pull request Oct 17, 2023

[MoveIt configs] use joint limits from description #668

Draft

Update README.md

2acca66

Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>

fmauch reviewed Oct 17, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

59d1819

gavanderhoorn reviewed Nov 23, 2023

View reviewed changes

gavanderhoorn approved these changes Nov 23, 2023

View reviewed changes

fmauch and others added 4 commits November 24, 2023 06:44

Remove duplicated entry.

32ee71a

Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>

README: Improve wording

d4217fb

Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>

Update ur20 MoveIt joint limits

f3c4285

Update README.md

0b99522

Added clarifying text reading the UR license to the Readme

fmauch merged commit cdf646b into ros-industrial:noetic-devel Dec 18, 2023
4 checks passed

urrsk deleted the ur20 branch December 19, 2023 12:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UR20 description and meshes #657

UR20 description and meshes #657

urrsk commented Sep 7, 2023

fmessmer commented Sep 25, 2023

fmauch left a comment

fmauch left a comment

RobertWilbrandt commented Oct 9, 2023

fmauch commented Oct 9, 2023

gavanderhoorn left a comment

urrsk commented Oct 16, 2023

fmauch commented Oct 30, 2023

fmessmer commented Nov 6, 2023

fmauch commented Nov 6, 2023

urrsk commented Nov 14, 2023

gavanderhoorn left a comment

gavanderhoorn Nov 23, 2023

gavanderhoorn Nov 23, 2023 •

edited

Loading

fmauch Nov 24, 2023

gavanderhoorn Nov 24, 2023 •

edited

Loading

gavanderhoorn Nov 24, 2023

urrsk Nov 24, 2023

gavanderhoorn commented Nov 23, 2023

fmessmer commented Dec 10, 2023

fmauch commented Dec 18, 2023

		[![License](https://img.shields.io/badge/License-BSD%203--Clause-blue.svg)](https://opensource.org/licenses/BSD-3-Clause)

		[![License](https://img.shields.io/badge/License-Universal%20Robots%20A/S%E2%80%99%20Terms%20and%20Conditions%20for%20Use%20of%20Graphical%20Documentation-blue.svg)](https://www.universal-robots.com/legal/terms-and-conditions/terms_and_conditions_for_use_of_graphical_documentation.txt)

UR20 description and meshes #657

UR20 description and meshes #657

Conversation

urrsk commented Sep 7, 2023

fmessmer commented Sep 25, 2023

fmauch left a comment

Choose a reason for hiding this comment

fmauch left a comment

Choose a reason for hiding this comment

RobertWilbrandt commented Oct 9, 2023

fmauch commented Oct 9, 2023

gavanderhoorn left a comment

Choose a reason for hiding this comment

urrsk commented Oct 16, 2023

fmauch commented Oct 30, 2023

fmessmer commented Nov 6, 2023

fmauch commented Nov 6, 2023

urrsk commented Nov 14, 2023

gavanderhoorn left a comment

Choose a reason for hiding this comment

gavanderhoorn Nov 23, 2023

Choose a reason for hiding this comment

gavanderhoorn Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

fmauch Nov 24, 2023

Choose a reason for hiding this comment

gavanderhoorn Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

gavanderhoorn Nov 24, 2023

Choose a reason for hiding this comment

urrsk Nov 24, 2023

Choose a reason for hiding this comment

gavanderhoorn commented Nov 23, 2023

fmessmer commented Dec 10, 2023

fmauch commented Dec 18, 2023

gavanderhoorn Nov 23, 2023 •

edited

Loading

gavanderhoorn Nov 24, 2023 •

edited

Loading