Gcode alignment #49
Gcode alignment #49
Conversation
|
I think I also included my typing system refactoring so the PR looks larger than I would like. Should I create separate PR for that as well to make this one more manageable? @voneiden |
|
Yeah, there are some conflicts as well in routers.py which prevents the CI checks from running. Give it a shot, feel free to split into smaller PR's if it makes sense. Smaller pieces are easier to review. And bonus points if you don't include any "Merge branch 'voneiden:unstable' into operation-refactor" commits in the PR. Not a hard requirement - I'll just skip it myself when merging. Technically nothing wrong with merging back and forth in git, but I prefer to keep the history easier to follow by allowing merges only one way (features -> main). |
|
Probably the easiest way would be to create a new branch from voneiden:unstable and then cherry-pick the commits you want to feature in the PR. You'll probably also want to (hard) reset your unstable branch to voneiden:unstable once you're sure that you're not accidentally wiping any of your own changes. |
|
I am doing all the development in their own branches so it shouldn' be too hard! |
|
I think router.py is generating unnecessary commands that were hidden by print_modal, print_xyz, and print_ijk. Now that I am simplifying those methods I am finding duplicate G1 and G0 commands. I think this approach is going to be good to uncover hidden bugs!! |
e3c4fda to
78e9f9b
Compare
Codecov Report
@@ Coverage Diff @@
## unstable #49 +/- ##
============================================
+ Coverage 84.30% 85.08% +0.77%
============================================
Files 37 39 +2
Lines 2568 2729 +161
============================================
+ Hits 2165 2322 +157
- Misses 403 407 +4
|
78e9f9b to
bd197d0
Compare
bd197d0 to
ea365e6
Compare
|
I am not sure what is actionable on the unresolved conversation? Is this good for now? @voneiden |
e26872c to
502e7a3
Compare
|
The FIXME and TODO notes currently still in routers.py are probably (?) not relevant? There's a circle offsetting bug in OCCT which cadquery suffers from but AFAIK there's no intention for the time being to apply the FreeCAD workaround ( CadQuery/cadquery#896 ) |
|
I have a local fix for that I have been using for quite a while that completely solves the issue. I can create a cadquery PR soonish! |
|
Kudos, SonarCloud Quality Gate passed!
|
|
This is ready to be merged! @voneiden |
|
Cheers! |
|
You are welcome! I almost have the next PR ready as well 🤣 |
I think I have made quite a few change. None of them are breaking changes just yet so I thought I can create a PR so that we can review and make our lives easier. If you feel I should add more changes to it then you let me know!
Closes #42