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

3.4 NOCHANGE: hangprinter: simplify things by turning unrolled stuff into loops #584

Closed
wants to merge 2 commits into from

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Sep 6, 2022

This is in preparation of potentially supporting more anchors for different hangprinter setups than the "standard" one, (4 anchors, 3 base anchors plus one on top for stability).
pinging @tobbelobb to have an opinion on the changes if possible.

There is no functional change in this PR (unless it has bugs).

@tobbelobb
Copy link
Contributor

I like this change. The unrolls were only a coincidence, I had no reason to unroll. I like all changes that bring us closer to Hangprinter's number of axes being a config.g variable.

I'd like to see a test on hardware or at least some kind of typo-safeguarding test before merge.

Otherwise it gets too deep.
I think there's an option in eclipse to forbid this by project in
format, one can config depness iirc.
@jtimon
Copy link
Contributor Author

jtimon commented Sep 12, 2022

I pushed a new version with improvements from the feedback and testing.

I tested M669 and M666 without parameters, but I still didn't manage to test HangprinterKinematics::WriteCalibrationParameters, how do I test that?

I also didn't test any of the modified parts related to CAN/Odrive. And I won't manage to test anytime soon, but at the same time those changes seem quite simple and inoffensive.

Testers on that front are very welcomed. For this PR, you just need a working hangprinter, upgrade to this build, test everything you normally do keeps working, report and go back to your previous working .bin, just in case.

Some parts can be tested without a hangprinter too, but we really don't want to break any of the current working hangprinters before we add any new experimental functionality.

@tobbelobb
Copy link
Contributor

HangprinterKinematics::WriteCalibrationParameters, how do I test that?

I think ran M500 and inspected the resulting sys/config-override.g manually. You could also do a M501 to make the machine try to read and load the stored parameters. Remember to delete sys/config-override.g when done.

Testers on that front are very welcomed. For this PR, you just need a working hangprinter, upgrade to this build, test everything you normally do keeps working, report and go back to your previous working .bin, just in case.

I'll see if I can put it on my machine within a couple of weeks. Will report back here.

@jtimon
Copy link
Contributor Author

jtimon commented Jan 27, 2023

Thank you. But, actually, perhaps for your testing you should wait for the rebased version of this PR, since more things changed than I expected. Some of the changes here may no longer needed. I will post a reference here to the new one and close this one when it's done.

@jtimon jtimon changed the title NOCHANGE: hangprinter: simplify things by turning unrolled stuff into loops 3.4 NOCHANGE: hangprinter: simplify things by turning unrolled stuff into loops Jan 30, 2023
@jtimon
Copy link
Contributor Author

jtimon commented Jan 30, 2023

Closing in favor of #597

@jtimon jtimon closed this Jan 30, 2023
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.

2 participants