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

Docs: sample-macros.cfg: minor change to avoid confusion #6528

Closed
wants to merge 2 commits into from
Closed

Docs: sample-macros.cfg: minor change to avoid confusion #6528

wants to merge 2 commits into from

Conversation

JamesH1978
Copy link
Collaborator

It has been noted a couple of times that this file states to only use the bare macro names in the slicer. Which is fine if you want to set your own temperatures in klipper but not if you want to pass the actual temperatures through as mentioned in the slicers.md doc linked. This minor change is aimed at causing less confusion.

Thanks
James

It has been noted a couple of times that this file states to only use the bare macro names in the slicer. Which is fine if you want to set your own temperatures in klipper but not if you want to pass the actual temperatures through as mentioned in the slicers.md doc linked. This minor change is aimed at causing less confusion.

Thanks
James

Signed-off-by: James Hartley <[email protected]>
typo

Signed-off-by: James Hartley <[email protected]>
@KevinOConnor
Copy link
Collaborator

Thanks. I'm not sure I understand the new comments.

Maybe an alternative would be to change the existing reference to Slicers.md to something like "Be sure to read docs/Slicers.md before using these macros."

Cheers,
-Kevin

@JamesH1978
Copy link
Collaborator Author

Basically some people were not either reading slicers.md and not realizing that to actually get the temperatures passed from a slicer they had to actually pass through the variables/parameters in the slicer gcode(and not reading the linked doc), Or they were viewing it as ignoring the sliceres temperature and just setting the defaults to what they want in the macro as defaults. This change is just to explain this and point them towards the file that was already pointed to for further information.

Does that make any more sense?

Thanks
James

@KevinOConnor
Copy link
Collaborator

I understand. However, I fear if I did not already know what the macros did, I would not understand the new comments.

I guess I'm wondering if it is useful to explain the low-level implications of the macros to those that don't want to read the documentation - as they wont read/understand these new comments either. Thus maybe we just need to make it more succinct that reading the documentation is necessary.

Thanks for working on this. We can certainly reword the comments and I'm open to any wording.
-Kevin

@JamesH1978
Copy link
Collaborator Author

The other option we have is we just put the correct syntax including the parameter pass in the sample macro file and tell them to read the documentation for more information.

Thanks
James

@flowerysong
Copy link
Contributor

Maybe something more like:

# START_PRINT and END_PRINT are intended to be called from your slicer's start and end g-code.
# See docs/Slicers.md for more information on using these macros.

It avoids the confusing "replace" language, but doesn't attempt to duplicate the documentation or go into excessive detail about the limitations of a specific way of calling the macros that isn't the way most people should be using them.

Copy link

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

  1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review
    If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
  2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
  3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@KevinOConnor
Copy link
Collaborator

The other option we have is we just put the correct syntax including the parameter pass in the sample macro file and tell them to read the documentation for more information.

That's fine.

We could also just set the temperatures explicitly in the sample macros. That is, if users are confused by the complexity of the example then we can simplify the example.

I'm open to other alternatives as well.

Cheers,
-Kevin

@JamesH1978 JamesH1978 closed this Aug 9, 2024
@JamesH1978 JamesH1978 deleted the patch-3 branch August 9, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants