Attempt to fix part of #2504

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
poserge
Posts: 58
Joined: Mon Apr 06, 2015 4:06 am

Attempt to fix part of #2504

Postby poserge » Thu Mar 09, 2017 5:13 am

Hi,

I'm trying to fix #2504 but I'm not sure if it is correct or not.
Bug report consist of two parts:
1. undo after migrate is not working properly
2. Revolve and Groove give errors when applied to a face.
This PR fixes second part and it is done by not allowing Revolve and Groove to be applied to faces.

I've read in old thread that they are supposed to work on faces but I'm not sure if it still should be possible.
This is the link to PR

Thanks for review.

Update: if I should not post each and every small change I'm making please let me know or just move this post somewhere else. I just feel bad when something doesn't work and I know I can fix it.
C++ beginner, willing to help FC with coding. Using FC for converting/decomposing STEP models into iges.
ickby
Posts: 2414
Joined: Wed Oct 05, 2011 7:36 am

Re: Attempt to fix part of #2504

Postby ickby » Thu Mar 09, 2017 8:43 am

Havin a seperate discussion if you have question is good, that makes it seperated clearly. If you are sure how to do things you don't need to ask, but if unsure the current workflow works well.

To the problem:
Revolve/Groove should work with faces too. They are also ProfileBased, and that class should theoretically abstract all differences between sketch and face away. We should investigate why Revolve/Groove are failing. Simply avoiding faces is not a good fix.
ickby
Posts: 2414
Joined: Wed Oct 05, 2011 7:36 am

Re: Attempt to fix part of #2504

Postby ickby » Thu Mar 09, 2017 8:45 am

Note that the linked bug also shows some bad behavior for drag/drop. I fixed this already in a local branch, so no need to put work into this topic.
poserge
Posts: 58
Joined: Mon Apr 06, 2015 4:06 am

Re: Attempt to fix part of #2504

Postby poserge » Thu Mar 09, 2017 12:59 pm

Thanks, I'll investigate.

About drag-n-drop -i remember I've fixed something similar http://forum.freecadweb.org/viewtopic.php?f=27&t=18975. Is the bug still there?
C++ beginner, willing to help FC with coding. Using FC for converting/decomposing STEP models into iges.
ickby
Posts: 2414
Joined: Wed Oct 05, 2011 7:36 am

Re: Attempt to fix part of #2504

Postby ickby » Thu Mar 09, 2017 1:04 pm

poserge wrote:About drag-n-drop -i remember I've fixed something similar http://forum.freecadweb.org/viewtopic.php?f=27&t=18975. Is the bug still there?

Ah no, your fix from back then works well. I fixed a few other drag n drop bugs in my branch. I'm actually not sure if the one mentioned in the bug report is still there, can't test right now. I just wanted to ensure that you do not spent too much time to fix drag n drop bus while I have done alot on this front already.
poserge
Posts: 58
Joined: Mon Apr 06, 2015 4:06 am

Re: Attempt to fix part of #2504

Postby poserge » Tue Mar 14, 2017 3:21 am

Hi,

I think I've fixed Revolve and Groove to accept faces. Here's the link to PR.

P.S. I've opened new PR on this which is probably not the best way of doing PRs.. Should have probably re-opened the initial one and augmented with new commits..
C++ beginner, willing to help FC with coding. Using FC for converting/decomposing STEP models into iges.
ickby
Posts: 2414
Joined: Wed Oct 05, 2011 7:36 am

Re: Attempt to fix part of #2504

Postby ickby » Tue Mar 14, 2017 6:10 am

this looks good, thanks for digging into it. One question: could you add a unit test for this behavior to part design? Lately we have a lot of failing functionality that worked once, as PartDesign is complex with all its selections and rules etc. this can happen easily, and I'm a bit afraid that we will not manage to keep such things running if there are no unit tests.
poserge
Posts: 58
Joined: Mon Apr 06, 2015 4:06 am

Re: Attempt to fix part of #2504

Postby poserge » Wed Mar 15, 2017 2:38 pm

I agree that tests are required, I'll add them. Thanks for reviewing.
C++ beginner, willing to help FC with coding. Using FC for converting/decomposing STEP models into iges.