-
Notifications
You must be signed in to change notification settings - Fork 538
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
Conversation
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. |
eedc8d0
to
c565aeb
Compare
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.
c565aeb
to
7f8ee5b
Compare
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. |
I think ran
I'll see if I can put it on my machine within a couple of weeks. Will report back here. |
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. |
Closing in favor of #597 |
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).