Issue27 configure link

[kazoo-kmt]

Add configuration from link to base/tool0. The logic is simply to delete existing link and add new link named base/tool0. May need to set some condition such as setDisable(true) for visual, collision, inertia

[gavanderhoorn]

Thanks for the PR.

Could you please remove all the Qt Creator files? They don't need to be committed to the repository. Specifically, these files should be removed:

CAD-to-ROS.config
CAD-to-ROS.creator
CAD-to-ROS.creator.user
CAD-to-ROS.files
CAD-to-ROS.includes

You can use git rm /path/to/file for that.

[kazoo-kmt]

Delete qt creator files. Also, added configuration for TCP (issue28).

[gavanderhoorn]

Delete qt creator files

thanks for deleting the files.

Also, added configuration for TCP (issue28).

I'd preferred it if you'd submitted a separate PR for #28, but let's keep it like this for now.

[kazoo-kmt]

Sorry, I’ll do that next time. Thanks!

On Apr 26, 2016, at 10:04, G.A. vd. Hoorn [email protected] wrote:

Delete qt creator files

thanks for deleting the files.

Also, added configuration for TCP (issue28).

I'd preferred it if you'd submitted a separate PR for #28 #28, but let's keep it like this for now.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub #84 (comment)

[gavanderhoorn]

@kazoo-kmt: I've checked your implementation again.

Could I ask you to revert 53c8271 (or at least the changes to urdf_editor/src/urdf_property.cpp)? Properly implementing #28 probably requires some more work, and I'd like to merge the other functionality in this PR (adding base and tool0 frames).

Some additional comments: TCP frames aren't necessarily always called TCP, so we should give the user the opportunity to at least provide a different name. The main point is though that #28 asks for a convenient way to add (multiple) TCP frames, which should probably include a dialogue (or wizard even) which gathers the required input from the user (name, transform, location, etc) and then adds & configures the new link & joint.

[gavanderhoorn]

Would this leak the QTreeItemWidget?

[kazoo-kmt]

I reverted the commit 53c8271.

[kazoo-kmt]

Thank you for other comments too. In order to change from "deleting and making a new link" to "just modifying", I need you advice again. Are there any classes that I can use to change the link's property?

[gavanderhoorn]

@kazoo-kmt wrote:

I reverted the commit 53c8271.

I think you now also reverted the changes to the Qt creator files. Could you remove those again?

Thank you for other comments too. In order to change from "deleting and making a new link" to "just modifying", I need you advice again. Are there any classes that I can use to change the link's property?

I think you'll want to look at LinkProperty (and perhaps JointProperty). You'll still need to manage the urdf link and add any required joints too.

Perhaps #97 can help here. It should make adding/removing links much easier.

[kazoo-kmt]

@gavanderhoorn deleted qtcreator files. I still need to make changes for dangling links based on your advice. Will do later.

[kazoo-kmt]

@gavanderhoorn I pulled the latest one and found there were a lot of changes. But I couldn't find where "tool0" or "TCP" were written in the file, even though I could see these menus when I opened urdf_builder. Do you know where are they? Sorry for a very simple question.

[AndyZe]

From the terminal, try "grep -r 'tool0' *"
On May 11, 2016 1:12 AM, "kazoo-kmt" [email protected] wrote:

@gavanderhoorn https://github.com/gavanderhoorn I pulled the latest one
and found there were a lot of changes. But I couldn't find where "tool0" or
"TCP" were written in the file, even though I could see these menus when I
opened urdf_builder. Do you know where are they? Sorry for a very simple
question.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#84 (comment)

[kazoo-kmt]

@AndyZelenak Thank you. I tried, but it only shows the list of urdf files...

[gavanderhoorn]

@kazoo-kmt: I'm not quite sure what it is you are asking. There are no references to tool0 or TCP in indigo-devel, as your PR hasn't been merged. If I gave that impression, I apologise.

[gavanderhoorn]

Issue #27 basically asks for a way to convert an existing link into a link with no child tags (ie: an empty <link .. />), and to set the name to tool0 or base, all in a single user action. The joint that connects that link to the rest of the tree should be left alone (as it's conceivable that users configure the joint first, and then convert the link to a tool0. Removing the joint would remove the transform the user just configured).

Your current implementation removes the link itself, which is different from conversion. In addition, it doesn't take care of attaching the replacement link to the tree (ie: there is no joint). That can't work. Finally, the current implementation also doesn't create a corresponding JointProperty, nor does it remove/update the joint (that previously connected the old link to the tree) from the model. Note that if you actually only update the link to be a tool0 or base link, the joint (and JointProperty) would not need any changes (other than those necessitated by the link name change).

These things will need to be fixed before we can merge.

[kazoo-kmt]

@gavanderhoorn After "git pull" recent changes, Do I need to build again in order to use the latest CAD-to-ROS software?

[gavanderhoorn]

Yes.

[gavanderhoorn]

@kazoo-kmt: we have our deadline coming up tomorrow. Do you think you can correct the outstanding issues by then?

[kazoo-kmt]

@gavanderhoorn Will try. I need to understand the structure of the program again because there are a lot of changes..

[gavanderhoorn]

You'll probably want to base your work on #97, as that is what the infrastructure for adding/removing/changing links will look like in the application once it's merged.

[kazoo-kmt]

@gavanderhoorn see. When will it be merged?

Also, in order to change the link name, I'm trying to use "on_propertyWidget_linkNameChanged(LinkProperty_,QVariant))". For LinkProperty_, I can use "ltree_to_link_property_[sel].get()", but for QVariant, I cannot use the string such as "tool0". How can I convert a string to QVariant? Or, is this wrong to use "on_propertyWidget_linkNameChanged" in the first place?

[gavanderhoorn]

@kazoo-kmt wrote:

@gavanderhoornI see. When will it be merged?

the planning is later today. But you don't need to wait for that: you can just check it out and branch of it in your workspace and work from that.

Also, in order to change the link name, I'm trying to use on_propertyWidget_linkNameChanged(LinkProperty*,QVariant)). For LinkProperty*, I can use ltree_to_link_property_[sel].get(), but for QVariant, I cannot use the string such as "tool0". How can I convert a string to QVariant? Or, is this wrong to use on_propertyWidget_linkNameChanged in the first place?

No, that is not really the idea. Qt slots are essentially event handlers, that get called whenever the corresponding signal is emitted. See URDFProperty::addLinkProperty(..) for an example of this (here is the 'subscription' to the event, and here is the 'publication' of the event).

To change the name of a link, it should be sufficient to assign a new name to the link (via its LinkProperty) and the emit the appropriate signal. One possibility would be to extend LinkProperty with a setLinkName(..) method, that first changes the name of it's associated link, and then emits the LinkProperty::linkNameChanged(..) signal. That should invoke all slots that have registered for that signal.

An alternative might be to get LinkProperty to change its "Name" property, and then emit the corresponding signal.

So on_propertyWidget_linkNameChanged(..) is a slot, which you typically don't call yourself, as you don't know who / what else might be interested in the event.

[kazoo-kmt]

Thank you for your comment. I couldn't fetch & checkout Levi's branch, so will continue on the current file.

[gavanderhoorn]

Thank you for your comment. I couldn't fetch & checkout Levi's branch, so will continue on the current file.

in your clone:

git remote add larmstrong https://github.com/Levi-Armstrong/CAD-to-ROS.git
git fetch larmstrong

# make a local branch to work on issue 27
git checkout -b base_tool0_links larmstrong/restructure

At this point you will work in a branch called base_tool0_links that will depart from where Levi's work currently is. You could also decide to work on-top of origin/m1_merge_candidate.

If you push that to your github fork, it will include Levi's work as well, so it will make merging it easier as well.

But if your changes are localised enough (ie: only in a few files), then continuing as you are now will probably also work.

[kazoo-kmt]

Thank you very much. I tried to make remote, but it returns "fatal: unable to access 'https://github.com/Levi-Armstrong/CAD-to-ROS.git/'”. Will find the answer.

Kazu Komoto
Make School ‘17
https://www.linkedin.com/in/kazukomoto

On May 14, 2016, at 01:53, G.A. vd. Hoorn [email protected] wrote:

Thank you for your comment. I couldn't fetch & checkout Levi's branch, so will continue on the current file.

in your clone:

git remote add larmstrong https://github.com/Levi-Armstrong/CAD-to-ROS.git
git fetch larmstrong

make a local branch to work on issue 27

git checkout -b base_tool0_links larmstrong/restructure
At this point you will work in a branch called base_tool0_links that will depart from where Levi's work currently is. You could also decide to work on-top of origin/m1_merge_candidate.

If you push that to your github fork, it will include Levi's work as well, so it will make merging it easier as well.

But if your changes are localised enough (ie: only in a few files), then continuing as you are now will probably also work.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #84 (comment)

[gavanderhoorn]

On 05/14/2016 08:57 PM, kazoo-kmt wrote:

Thank you very much. I tried to make remote, but it returns "fatal: unable to access 'https://github.com/Levi-Armstrong/CAD-to-ROS.git/'”. Will find the answer.

ah, yes. I keep forgetting. My apologies: you cannot access Levi's remote, only repository owners can in the current setup.

No problem: just try to work with m1_merge_candidate in stead. That is on origin, so should be accessible to you.

[kazoo-kmt]

Thank you very much. I gave up to use the following branch and continue to develop on the original one. I believe my change is not many, so easy to change later.

Inside void URDFProperty::on_treeWidget_customContextMenuRequested(const QPoint &pos) function, I’m trying to change name and delete parameter, but couldn’t find how to solve yet.

// 1. Access name property and change name
          QString new_name = "tool0"     
          QString orig_name = sel->text(0);
          int idx = link_names_.indexOf(orig_name);
          link_names_.replace(idx, new_name);
          sel->setText(0, new_name);

// 2. Emit the corresponding response
          const QVariant val;
          emit LinkProperty::linkNameChanged(activeLink.get(), &val);

// 3. Access other properties and delete them
// 4. Emit the corresponding response

After step 2, I got errors like the following:

cannot call member function ‘void urdf_editor::LinkProperty::linkNameChanged(urdf_editor::LinkProperty*, const QVariant&)’ without object
           emit LinkProperty::linkNameChanged(activeLink.get(), &val);

[gavanderhoorn]

I don't think it's necessary to update link_names_ yourself, that is already done in URDFProperty::on_propertyWidget_linkNameChanged(..) (here), provided you emit the appropriate signal.

You might just have to change the Name property of the link, and then trigger the signals.

re: removing current properties: see here for an example.

Note that you'll have to patch up the relevant urdf model objects as well.

[kazoo-kmt]

Oh, did you mean I need to change the name in the property browser and no need to change the name on widget tree?
Also, current problem is that emit LinkProperty::linkNameChanged(activeLink.get(), &val); doesn't work as written above. What kinds of argument do I need to use?

[kazoo-kmt]

Please forget the previous comment. As you suggested before, now I wrote

          std::string new_name = "tool0";
          LinkPropertySharedPtr activeLink = ltree_to_link_property_[sel];
          activeLink->setLinkName(new_name);

in urdf_property.cpp. Then, I wrote setLinkName function.

void LinkProperty::setLinkName(std::string new_name)
  {
    link_->name = new_name;
    LinkProperty::valueChanged();
    emit LinkProperty::linkNameChanged(this, new_name);
  }

It can change the name in link property. I'm still confused how to use emit. for the second argument of LinkProperty::linkNameChanged(this, new_name);, I need QVariant type. However, I don't understand how where to get QVariant type variable or how to convert to QVariant type..

Sorry for many questions. I really appreciate your help.

[Levi-Armstrong]

@kazoo-kmt, Thank you for your help. Do you mind re-basing using m1_merge_candidate_v2? If I did not answer all of your question below let me know.

void LinkProperty::setLinkName(std::string new_name)
  {
    link_->name = new_name;
    LinkProperty::valueChanged(); 
    emit LinkProperty::linkNameChanged(this, new_name);
  }

The method below just calls a signal which does not perform any operation. If you are intending to set the value, use the name_item_->setValue(new_name). You should not need to emit a signal this will be handled by the property widget when using the setValue.

LinkProperty::valueChanged(); 

One other thing for consistency please us QString in place of std::string. QString provides the ability to convert to standard string if need. This will also solve the QVariant question since QString can be passed as a QVariant without any additional work.

[kazoo-kmt]

@Levi-Armstrong Thank you very much. Don't I need permission to rebase m1_merge_candidate_v2? In the past, I tried to fetch and merge m1_merge_candidate_v1, but it didn't work.

[gavanderhoorn]

@kazoo-kmt wrote:

Don't I need permission to rebase m1_merge_candidate_v2? In the past, I tried to fetch and merge m1_merge_candidate_v1, but it didn't work.

Levi asks you to rebase your own work (so kazoo-kmt/CAD-to-ROS-1/issue27-configure-link), not m1_merge_candidate_v2. You have pull access to m1_merge_candidate_v2, so you should be able to do that.

[kazoo-kmt]

@gavanderhoorn @Levi-Armstrong Thank you very much. Just rebased and sent a new pull request.