Pull request discussion for Path postprocessor tests
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
-
- Posts: 98
- Joined: Fri Oct 09, 2020 4:56 pm
- Location: Oregon, USA
Pull request discussion for Path postprocessor tests
I would like to create a pull request for some code that gets the Path postprocessor tests working and adds a few new tests.
This should go into the next release (0.21 or whatever it ends up being called).
This is my first pull request so please point out what I need to do differently
I will create the pull request in "Draft" mode, then update this message to include the link.
https://github.com/FreeCAD/FreeCAD/pull/6535
This should go into the next release (0.21 or whatever it ends up being called).
This is my first pull request so please point out what I need to do differently
I will create the pull request in "Draft" mode, then update this message to include the link.
https://github.com/FreeCAD/FreeCAD/pull/6535
-
- Posts: 98
- Joined: Fri Oct 09, 2020 4:56 pm
- Location: Oregon, USA
Re: Pull request discussion for Path postprocessor tests
I have created a new pull request (#7007) which replaces the previous pull request (#6894).
The new request is still in Draft and is a work in progress. It contains everything that was in PR6894 plus three new refactored postprocessors and now-common code without removing any of the existing postprocessors or support files. I still need to refactor some of the now-shared code some more as well as refactor the remaining postprocessors, then write many more unit tests.
This is for the 0.21 release and is posted now to make review easier.
The new request is still in Draft and is a work in progress. It contains everything that was in PR6894 plus three new refactored postprocessors and now-common code without removing any of the existing postprocessors or support files. I still need to refactor some of the now-shared code some more as well as refactor the remaining postprocessors, then write many more unit tests.
This is for the 0.21 release and is posted now to make review easier.
Re: Pull request discussion for Path postprocessor tests
Moved from Open discussion forum.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
-
- Posts: 98
- Joined: Fri Oct 09, 2020 4:56 pm
- Location: Oregon, USA
Re: Pull request discussion for Path postprocessor tests
Pull Request #7007 has now been merged. It adds 4 "refactored" versions of the centroid, grbl, linuxcnc, and mach3/4 postprocessors along with their supporting files and tests. This should not affect the rest of FreeCAD at all. It would only matter if someone chose to try running one of the "refectored" postprocessors.
This code is still being refactored and developed at this time, so I would rate the "refactored" postprocessors as "experimental". They should work and any testing would be welcome, but the implementation and capabilities may change at any time, so don't build any dependencies on them just yet
I have created Pull Request #7185 which adds even more tests and refactors the TestRefactoredTestPost.py file so there is much less "copy and paste" code, reducing the size of the file significantly. It also adds the ability to "decompose" G73 G-code into G0 and G1 G-code commands.
https://github.com/FreeCAD/FreeCAD/pull/7185
This code is still being refactored and developed at this time, so I would rate the "refactored" postprocessors as "experimental". They should work and any testing would be welcome, but the implementation and capabilities may change at any time, so don't build any dependencies on them just yet
I have created Pull Request #7185 which adds even more tests and refactors the TestRefactoredTestPost.py file so there is much less "copy and paste" code, reducing the size of the file significantly. It also adds the ability to "decompose" G73 G-code into G0 and G1 G-code commands.
https://github.com/FreeCAD/FreeCAD/pull/7185
-
- Posts: 98
- Joined: Fri Oct 09, 2020 4:56 pm
- Location: Oregon, USA
Re: Pull request discussion for Path postprocessor tests
Pull Request #7185 has been merged.
I have created Pull Request #7826 which will incorporate changes based on feedback from code reviews.
https://github.com/FreeCAD/FreeCAD/pull/7826
I have created Pull Request #7826 which will incorporate changes based on feedback from code reviews.
https://github.com/FreeCAD/FreeCAD/pull/7826
Re: Pull request discussion for Path postprocessor tests
Just getting back to FreeCAD Path after a few month absence. I had created a working post processor for Dynapath Delta 40-60 controls. Now of course with the latest FreeCAD v21, it's broken. I'm now wanting to get it updated to work with the latest changes. Are there any tips or suggestions you can provide for getting it "refactored" and working with v21?
Thanks
Dan
Thanks
Dan
-
- Posts: 98
- Joined: Fri Oct 09, 2020 4:56 pm
- Location: Oregon, USA
Re: Pull request discussion for Path postprocessor tests
My first suggestion is to ignore the "refactored" postprocessors for now. They will be going through a lot of changes.
There was a rearrangment of the Path directory structure a couple of months ago that required some changes to the "import" commands at the front of the postprocessor files. I suggest you look at the "linuxcnc_post.py" file (for example) to see how the import commands changed.
Other than that, I don't know of any reason that a postprocessor file that was working on versions 19 or 20 shouldn't work on version 21.
There was a rearrangment of the Path directory structure a couple of months ago that required some changes to the "import" commands at the front of the postprocessor files. I suggest you look at the "linuxcnc_post.py" file (for example) to see how the import commands changed.
Other than that, I don't know of any reason that a postprocessor file that was working on versions 19 or 20 shouldn't work on version 21.
Re: Pull request discussion for Path postprocessor tests
Ahh, okay. I'll do some digging in to this. Thank you!LarryWoestman wrote: ↑Sun Dec 04, 2022 2:19 am My first suggestion is to ignore the "refactored" postprocessors for now. They will be going through a lot of changes.
There was a rearrangment of the Path directory structure a couple of months ago that required some changes to the "import" commands at the front of the postprocessor files. I suggest you look at the "linuxcnc_post.py" file (for example) to see how the import commands changed.
Other than that, I don't know of any reason that a postprocessor file that was working on versions 19 or 20 shouldn't work on version 21.
Re: Pull request discussion for Path postprocessor tests
@LarryWoestman, my custom post processor is working after updating the import statements, thanks for the tip! However, it appears any pp I select outside of the "refactored" ones complain with errors while accessing Path Preferences Post Processor tab. This error seems to be merely a nuisance for now and doesn't seem to break any functionality.
While digging in to this further, I noticed a comment you added to one of the refactored post processors that indicated a need for TOOLTIP, TOOLTIP_ARGS, & UNITS needed to be declared Global to keep the PathPostProcessor.load method happy. My custom pp is complaining about the "Load" method in Path Preferences and so is the non refactored linuxcnc_post. Here is a small sample of the errors. Curious if this was the error you were getting prior?
While digging in to this further, I noticed a comment you added to one of the refactored post processors that indicated a need for TOOLTIP, TOOLTIP_ARGS, & UNITS needed to be declared Global to keep the PathPostProcessor.load method happy. My custom pp is complaining about the "Load" method in Path Preferences and so is the non refactored linuxcnc_post. Here is a small sample of the errors. Curious if this was the error you were getting prior?
Code: Select all
09:29:19 File "/tmp/.mount_FreeCA0t0EeI/usr/Mod/Path/Path/Main/Gui/PreferencesJob.py", line 339, in setPostProcessorTooltip
09:29:19 processor = self.getPostProcessor(name)
09:29:19 File "/tmp/.mount_FreeCA0t0EeI/usr/Mod/Path/Path/Main/Gui/PreferencesJob.py", line 333, in getPostProcessor
09:29:19 processor = PostProcessor.load(name)
09:29:19 AttributeError: module 'Path.Post.Processor' has no attribute 'load'
-
- Posts: 98
- Joined: Fri Oct 09, 2020 4:56 pm
- Location: Oregon, USA
Re: Pull request discussion for Path postprocessor tests
That error message was familiar to me
After the Path file structure rearrangement I ran into some cases where I found "import Path.Post.Processor as PostProcessor", which doesn't work. It needs to be "from Path.Post.Processor import PostProcessor" instead. I fixed the cases where I noticed the problem.
I just searched through the entire Path subdirectory in my build tree and found one more case in Mod/Path/Path/Main/Gui/PreferencesJob.py. You might want to try changing that file in your installed version of FreeCAD and see if that helps. If it does let me know and I will file a Pull Request and issue to get it fixed. I don't usually use the GUI when testing my code so I probably would not have executed that file.
I just noticed that the exception is being thrown from that file, which makes it even more likely that that is where the problem is.
After the Path file structure rearrangement I ran into some cases where I found "import Path.Post.Processor as PostProcessor", which doesn't work. It needs to be "from Path.Post.Processor import PostProcessor" instead. I fixed the cases where I noticed the problem.
I just searched through the entire Path subdirectory in my build tree and found one more case in Mod/Path/Path/Main/Gui/PreferencesJob.py. You might want to try changing that file in your installed version of FreeCAD and see if that helps. If it does let me know and I will file a Pull Request and issue to get it fixed. I don't usually use the GUI when testing my code so I probably would not have executed that file.
I just noticed that the exception is being thrown from that file, which makes it even more likely that that is where the problem is.