RC stairs macro

three_d · Post by **three_d** » Sun Jan 24, 2021 8:05 pm

Hi, paullee thank you for taking the time to check out my work, I'm grateful to you.

As you already know, I attempted my first PR....but something went wrong!

GitHub reports that the test is failed.....and I still don't understand why.
Can someone please help me solve this problem? Many thanks.

Note: I developed code directly on GitHub account, so I have not worked locally on my computer.

My simple workflow on Github has been:
- I created a branch called "archstairs_connections_flight_landing", in my personal repo.
- In this new branch, I modified the ArchStairs.py file
- Last, I submitted the pull request.

balrobs · Post by **balrobs** » Mon Jan 25, 2021 1:06 am

Hi @tree_d,
I also tested your amazing work and have to say thank you for improving the stair tool this way.
My test case was similar to that of @paullee and worked well till I went into further detailing ... there I realized two minor issues:

when for flights Riser_Thickness>0 the connection points to the landings get messy
when for landings Riser_Thickness>0 FreeCad throws errors, the corresponding tree element shows an error symbol and no Riser_Thickness is applied to the landing.

: Screenshot from 2021-01-25 01-29-22.png (182.21 KiB) Viewed 1938 times

Here is my test file:

Stair_02.FCStd: (119.33 KiB) Downloaded 66 times

Post by **paullee** » Mon Jan 25, 2021 1:07 am

Everything seems fine, continue discussion on github. Hope it is merged soon

[EDIT]

Good testing and findings @balrobs ::D

Maybe fix in next PR ? (An issue on github / Mantis ?)

three_d · Post by **three_d** » Mon Jan 25, 2021 9:15 pm

Hi @balrobs,
glad, that you like it and thanks for the reporting!
I was focused on the connection between stair flight and landing and I haven't considered this aspect....I plan to implement the riser thickness too......in the meantime if anyone wants to try it and report back other possible cases where links do not work, is welcome!

paullee wrote: ↑Mon Jan 25, 2021 1:07 am Everything seems fine, continue discussion on github. Hope it is merged soon

Thanks paullee!!

balrobs · Post by **balrobs** » Tue Jan 26, 2021 7:59 am

three_d wrote: ↑Mon Jan 25, 2021 9:15 pm Hi @balrobs,
glad, that you like it and thanks for the reporting!
I was focused on the connection between stair flight and landing and I haven't considered this aspect....I plan to implement the riser thickness too......in the meantime if anyone wants to try it and report back other possible cases where links do not work, is welcome!

paullee wrote: ↑Mon Jan 25, 2021 1:07 am Everything seems fine, continue discussion on github. Hope it is merged soon
Thanks paullee!!

Hi @tree_d,
I also hope your work is merged soon. The "issues" I noticed are definitely a corner case and should not delay the merge.
Thank's again fot the great improvement!

three_d · Post by **three_d** » Tue Jan 26, 2021 8:39 pm

balrobs wrote: ↑Tue Jan 26, 2021 7:59 am Hi @tree_d,
I also hope your work is merged soon. The "issues" I noticed are definitely a corner case and should not delay the merge.
Thank's again fot the great improvement!

Hi @balrobs,
thanks for your encouragement!

I found another bug that occurs when you set stair property "Structure" to "One Stringer" or "Two Stringers", as you can see below:

: stair_geometry_fails[resize].png (158.55 KiB) Viewed 1805 times

.....at the moment, I have no idea how long it'll take to fix them....and helps and suggestions are always welcome!

Post by **paullee** » Wed Jan 27, 2021 6:40 pm

three_d wrote: ↑Tue Jan 26, 2021 8:39 pm I found another bug that occurs when you set stair property "Structure" to "One Stringer" or "Two Stringers", as you can see below:

Indeed. But it seems it only happens when the Flight is not Straight, i.e. HalfTurnLeft or HalfTurnRight, probably seldom people use that at the moment

Maybe a bug in mantis for further improvement.

: Screenshot from 2021-01-28 02-36-59.png (321.27 KiB) Viewed 1749 times

hyarion · Post by **hyarion** » Wed Jan 27, 2021 9:14 pm

paullee wrote: ↑Wed Jan 27, 2021 6:40 pm
three_d wrote: ↑Tue Jan 26, 2021 8:39 pm I found another bug that occurs when you set stair property "Structure" to "One Stringer" or "Two Stringers", as you can see below:
Indeed. But it seems it only happens when the Flight is not Straight, i.e. HalfTurnLeft or HalfTurnRight, probably seldom people use that at the moment

Maybe a bug in mantis for further improvement.

Are these issues due to three_d's code or can they be reproduced with an unmodified version of FreeCAD?

The reason I ask is that I don't think it's a good idea to merge features with known new bugs. It's generally better to fix those issues first, and then merge.
However, it is ok to leave old bugs, if that's the case, but then they should of cause be reported

Post by **paullee** » Thu Jan 28, 2021 12:23 am

hyarion wrote: ↑Wed Jan 27, 2021 9:14 pm Are these issues due to three_d's code or can they be reproduced with an unmodified version of FreeCAD?

The reason I ask is that I don't think it's a good idea to merge features with known new bugs. It's generally better to fix those issues first, and then merge.
However, it is ok to leave old bugs, if that's the case, but then they should of cause be reported

Hmmm... seems a corner case may defer fixing. Anyway, tested.

It seems it was a problem before @three_d, then it should be me when introducing the HalfTurn flight not tested every cornercase