Assembly: fix gears created with null radius and de-sync
This adapts the UI after connection all of the properties, fixing #17964, #17797 and #18163 where the radius were zero for gears but displayed 1 in the GUI.
Also fixes #17798 where reversed / non revered checkbox was not updated and editing existing joint.
Edit : found another issue that should be resolved
@PaddleStroke for review ?
@VincidaB thanks for looking into this ! Ok so this is very touchy. I actually modified the position of adaptUi on purpose to fix some bugs. And if I recall correctly I have done so several times. So it is very likely that by doing this you are solving those bugs, but re-introducing others.
So you need to take a close look at the history on this file.
For instance https://github.com/FreeCAD/FreeCAD/commit/28a977b5fcdc99d3a77ec5f8fbeb0fb0828a81ad is moving a lot of the connections after adaptUi (which is what you undo here).
I cannot remember precisely the issue but I seem to recall that it was a problem that the limits text would not update to show the current angle when they were deactivated. I seem to recall that the reason was that self.form.limitCheckbox1.stateChanged.connect(self.adaptUi) was before adaptUi which is setting the states of the checkboxes. So it would call again or something like this.
Could you please make an extended testing of the taskbox to make sure this is not reintroducing some bad behaviors? I will also have a closer look at it.
I figured that the order might be important but the limits still work :
- I can enable them
- They stay consistent when editing from the data
I don't get why on double click in the tree we call the constructor with jointTypeIndex = 0 :
https://github.com/FreeCAD/FreeCAD/blob/e2b36e583babbb630e97b7bc0192e697f317b141/src/Mod/Assembly/JointObject.py#L1189-L1190
https://github.com/FreeCAD/FreeCAD/blob/e2b36e583babbb630e97b7bc0192e697f317b141/src/Mod/Assembly/JointObject.py#L920-L933
~~While testing I also noticed an issue with angle limits where only 2 digits can be typed (decimals working) I will look into that~~
I don't get why on double click in the tree we call the constructor with jointTypeIndex = 0 :
@VincidaB because updateTaskboxFromJoint is setting the correct joint type already. Perhaps it's not the best. We could indeed pass the correct joint type to the ctor.
After a more detailed analysis, I think this change is OK. And it does fix those issues so that's great. Thanks again for looking into those!
And it does fix those issues so that's great.
Just keep in mind that only the last one got linked to this PR.