Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added command M181. #66

Merged
merged 4 commits into from Apr 22, 2023
Merged

Added command M181. #66

merged 4 commits into from Apr 22, 2023

Conversation

lromor
Copy link
Contributor

@lromor lromor commented Apr 16, 2023

  • Parser shall route to unprocessed commands.
  • GCodeMachineControl shall handle the command and set the value to max if no S pair is provided. Added single test that only checks the motor ops queue shall be emptied before changing the planner size.
  • Planner respect the lookahead value and always push older commands to maintain planning queue size. No tests added.

- Parser shall route to unprocessed commands.
- GCodeMachineControl shall handle the command and set the value to max if no S<value> pair is provided. Added single test that only checks the motor ops queue shall be emptied before changing the planner size.
- Planner respect the lookahead value and always push older commands to maintain planning queue size. No tests added.
@lromor lromor marked this pull request as draft April 16, 2023 12:59
@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.72 🎉

Comparison is base (4d55223) 45.93% compared to head (302fd25) 47.66%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
+ Coverage   45.93%   47.66%   +1.72%     
==========================================
  Files          37       37              
  Lines        4245     4284      +39     
==========================================
+ Hits         1950     2042      +92     
+ Misses       2295     2242      -53     
Impacted Files Coverage Δ
src/gcode-machine-control.cc 30.41% <100.00%> (+10.30%) ⬆️
src/planner.cc 83.74% <100.00%> (+0.79%) ⬆️

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@hzeller hzeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of comments.

src/planner.h Outdated Show resolved Hide resolved
src/planner.cc Outdated Show resolved Hide resolved
src/planner.cc Outdated Show resolved Hide resolved
src/gcode-machine-control.cc Outdated Show resolved Hide resolved
src/gcode-machine-control.cc Outdated Show resolved Hide resolved
src/gcode-machine-control.cc Outdated Show resolved Hide resolved
src/gcode-machine-control_test.cc Outdated Show resolved Hide resolved
src/planner.h Show resolved Hide resolved
src/gcode-machine-control_test.cc Outdated Show resolved Hide resolved
Copy link
Contributor Author

@lromor lromor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the requested changes. Now working on a test for the planner.

src/planner.cc Outdated Show resolved Hide resolved
src/planner.h Outdated Show resolved Hide resolved
src/gcode-machine-control.cc Outdated Show resolved Hide resolved
src/gcode-machine-control.cc Outdated Show resolved Hide resolved
src/planner.cc Outdated Show resolved Hide resolved
src/gcode-machine-control_test.cc Outdated Show resolved Hide resolved
src/gcode-machine-control_test.cc Outdated Show resolved Hide resolved
src/gcode-machine-control_test.cc Show resolved Hide resolved
@lromor
Copy link
Contributor Author

lromor commented Apr 19, 2023

@hzeller , I've added a planner test to check the lookahead resize behavior. Let me know what you think!

@lromor lromor marked this pull request as ready for review April 19, 2023 20:15
Copy link
Owner

@hzeller hzeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A line in the G-Code documentation would probably a good follow-up change.

@hzeller hzeller merged commit ed37f63 into hzeller:main Apr 22, 2023
17 checks passed
@hzeller
Copy link
Owner

hzeller commented Apr 22, 2023

In the gcode_finished() callback the GCodeMachineControl receives: might be useful to also reset the planner buffer to normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants