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.

Sign up for GitHub

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

Jump to bottom

Added board config for Mellow Fly E3 v2 #6682

Merged
merged 9 commits into from
Nov 7, 2024
Merged

Added board config for Mellow Fly E3 v2 #6682

merged 9 commits into from
Nov 7, 2024

Conversation

LievenVanh
Copy link
Contributor

Added new generic board configuration for Mellow Fly E3 v2

@JamesH1978
Copy link
Collaborator

Thankyou for submitting a PR, please can you sign off your as mentioned in point 3 in https://github.com/Klipper3d/klipper/blob/master/docs/CONTRIBUTING.md#what-to-expect-in-a-review and provide am update to the printer.test file as per point 4 in https://github.com/Klipper3d/klipper/blob/master/docs/Example_Configs.md#guidelines

Thanks
James

@LievenVanh
Added generic-mellow-fly-e3-v2.cfg to printers.test
6108958 
Included the new board file in the printers.test file.
Added copyright info in the board file

Signed-off-by: Lieven Vanhercke <[email protected]>
@LievenVanh
Copy link
Contributor Author

Thankyou for submitting a PR, please can you sign off your as mentioned in point 3 in https://github.com/Klipper3d/klipper/blob/master/docs/CONTRIBUTING.md#what-to-expect-in-a-review and provide am update to the printer.test file as per point 4 in https://github.com/Klipper3d/klipper/blob/master/docs/Example_Configs.md#guidelines

Thanks James

Sorry, I missed that completely, I'm pretty new to the collaboration stuff ;)
This should be fixed now

@LievenVanh
Merge branch 'Klipper3d:master' into master
e11eff1
@LievenVanh
Fixed commenting references to extruder1
128cc2a 
I had commented [extruder1] but had left the [tmc2209 extruder1] section active. Fixed now

Signed-off-by: Lieven Vanhercke <[email protected]>
@LievenVanh
Merge branch 'master' of https://github.com/LievenVanh/klipper
c9c4cb7
@LievenVanh
Update pid control on mellow-fly-e3-v2.cfg
ab51505 
Added pid control to extruder and heated bed

Signed-off-by: Lieven Vanhercke <[email protected]>
JamesH1978
JamesH1978 suggested changes Sep 13, 2024
View reviewed changes
Copy link
Collaborator

@JamesH1978 JamesH1978 left a comment

Choose a reason for hiding this comment

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

Please see my comments.

Thanks
James

config/generic-mellow-fly-e3-v2.cfg Outdated Show resolved Hide resolved
config/generic-mellow-fly-e3-v2.cfg Outdated
interpolate: False
run_current: 0.3
sense_resistor: 0.110
stealthchop_threshold: 500
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not set a non 0 or 999999 figure for stealthchop_threshold, the option should be on or off. Please see https://github.com/Klipper3d/klipper/blob/master/docs/TMC_Drivers.md#setting-spreadcycle-vs-stealthchop-mode

This goes for all tmc sections

Thanks
James

config/generic-mellow-fly-e3-v2.cfg Outdated
# LIGHTING
#######################################

[led Toolhead]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please comment this section out, as it is not known if the end user will use this option.

Thanks
James

config/generic-mellow-fly-e3-v2.cfg Outdated
max_z_velocity: 50
max_z_accel: 100

[safe_z_home]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This section is considered non-standard to a generic config, please remove it.

Thanks
James

config/generic-mellow-fly-e3-v2.cfg Outdated Show resolved Hide resolved
@LievenVanh
Update mellow-fly-e3-v2.cfg as per comments
b6e14ba 
Removed safety adaptations that are non-standard, fixed stealthchop_threshold values to valid ones, commented toolhead lighting section

Signed-off-by: Lieven Vanhercke <[email protected]>
@LievenVanh
Copy link
Contributor Author

all changes are made, sorry for the extra work I am giving you :/

@github-actions GitHub Actions
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

Hi. I saw your comment on Discourse asking for a review, but it seems not all of James' comments have been addressed.

Cheers,
-Kevin

@JamesH1978
Copy link
Collaborator

Apologies for the delay, i think everything i asked for has been addressed now. i will review again.

Thanks
James

JamesH1978
JamesH1978 approved these changes Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@JamesH1978 JamesH1978 left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Thanks
James

@KevinOConnor
Copy link
Collaborator

Oh - there is still the unusual copyright statement at the top of the config file. Not a huge deal either way, but seems odd.

Cheers,
-Kevin

@JamesH1978
Copy link
Collaborator

Oh, I couldnt see the statement, i even went to his fork for the PR and its not there, unless i am missing it @KevinOConnor

@LievenVanh
Update generic-mellow-fly-e3-v2.cfg
b42d2c6 
Removed extra copyright in config file

Signed-off-by: Lieven Vanhercke <[email protected]>
JamesH1978
JamesH1978 approved these changes Nov 4, 2024
View reviewed changes
Copy link
Collaborator

@JamesH1978 JamesH1978 left a comment

Choose a reason for hiding this comment

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

Thanks for the change, looks good.

Thanks
James

@KevinOConnor KevinOConnor merged commit a340344 into Klipper3d:master Nov 7, 2024
1 check passed
@KevinOConnor
Copy link
Collaborator

Thanks.

-Kevin

ejhoness added a commit to ejhoness/klipperBigtreetech that referenced this pull request Nov 7, 2024
@ejhoness
Merge pull request #20 from Klipper3d/master
712ab7c 
config: Added board config for Mellow Fly E3 v2 (Klipper3d#6682)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KevinOConnor KevinOConnor KevinOConnor left review comments

@JamesH1978 JamesH1978 JamesH1978 approved these changes

Assignees
No one assigned
Labels
reviewer needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LievenVanh @JamesH1978 @KevinOConnor