search

DeepSOIC / Lattice2

FreeCAD workbench about arrays of all sorts and kinds, and local coordinate systems
Other
53 stars 9 forks source link

fix DeepSOIC#86 Property container has no property Support in FreeCAD 0.22 > 36274 #87

Closed johnsonm closed 5 months ago

johnsonm commented 6 months ago

Following the suggested pattern in https://github.com/FreeCAD/FreeCAD/pull/12714#issuecomment-1985750506 handle the change from Support to AttachmentSupport in current development builds.

DeepSOIC commented 6 months ago

please excuse my complaints... 1) this is not the only place where Support property is being used. One more place would be a polar array attached to an arc of circle, where lattice2 inspects the Support prop to know the arc parameters. Could be more. 2) i have a special file for handling version checks, lattice2Compatibility.py. There is some complicated logic there, and it is there for reasons (FC wasn't always consistent with how it reports version, and some builds may be missing verion info altogether). We should make use of that. 3) i think, it might be better to just check if AttachmentSupport property exists first, and fall back to using Support for backwards compatibility. This avoids version checks altogether, which i think is good.

johnsonm commented 6 months ago

Those are not complaints, they are helpful! I won't be able to look at this for at least some days, and also have I no pride in my hack. So if you or someone else beat me to it, no harm to me. But also I intend to get back to this when I have time if no one else beats me to it.

johnsonm commented 6 months ago

I am a complete novice in lattice2 usage, so I don't know how to test effectively, but I tried to go through all instances of "Support" and do what looked right. There was one place where I didn't have an object to compare and have to fall back on revision, and I put that into lattice2Compatibility.py so that if it's needed again somewhere at least the logic is in one place.

Please feel free to "complain" in any way about my new approach, and no pressure from me to merge this or to respond on any particular schedule.

johnsonm commented 5 months ago

At least basic polar arrays appear to function without error in 0.21.2 and a recent weekly build with this change, and neither shows the error that set me off to fix this in the first place.

Because I'm still a novice with lattice2, that's probably all I can realistically do to validate this work myself.

DeepSOIC commented 5 months ago

looks pretty good now, thanks! i'll give it a quick test (later) and then merge

DeepSOIC commented 5 months ago

this pr makes me hate myself for not having unit tests... Hopefully everything is good now.

johnsonm commented 5 months ago

Ah, I guessed wrong in one place I see...