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.5 NOCHANGE: Hangprinter: simplify things by turning unrolled stuff into loops #597

Open
wants to merge 2 commits into
base: 3.5-dev
Choose a base branch
from

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Jan 30, 2023

This is a rebased version of #584 in the 3.4-dev branch.

The main goal is to make it easier to get support for more than 4 anchors in the hangprinter kynematics.
This PR shouldn't change functionality at all. If it does, that's a bug.

Testing status:

Everything seems to work, I still haven't tested the changes in WriteCalibrationParameters()
But not with a real hangprinter, my config is different with 4 anchors on top and not really ready to print anything, just moving the headtool around for now.
Real hangprinter testers welcomed.

@jtimon jtimon changed the title NOCHANGE: hangprinter: simplify things by turning unrolled stuff into loops 3.5 NOCHANGE: hangprinter: simplify things by turning unrolled stuff into loops Jan 30, 2023
@jtimon jtimon force-pushed the 3.5-dev-hangprinter-simplify branch from 049b547 to 44d22c2 Compare February 25, 2023 21:23
@jtimon
Copy link
Contributor Author

jtimon commented Feb 25, 2023

Updated the code and the testing status on the PR description. TLDR: now it compiles again.
Now it is not as complete as the one for version 3.4 because some of the new changes are not really accounted for yet.
Some new changes are naming anchors directly. I can solve it in this PR or another one.

@jtimon jtimon changed the title 3.5 NOCHANGE: hangprinter: simplify things by turning unrolled stuff into loops 3.5 NOCHANGE: Hangprinter: simplify things by turning unrolled stuff into loops Feb 25, 2023
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 jtimon force-pushed the 3.5-dev-hangprinter-simplify branch from 44d22c2 to e19697e Compare March 20, 2023 19:11
@jtimon
Copy link
Contributor Author

jtimon commented Mar 20, 2023

Finally tested this a bit. There were some bugs in M666 that are corrected now.
Updated the testing status in the description. I would say this one is pretty much ready, but if somebody can test it with a real hangprinter in a way that WriteCalibrationParameters gets tested too, that would be ideal.

@jtimon
Copy link
Contributor Author

jtimon commented Mar 20, 2023

Oh, well, I guess I also didn't test PrintParameters().
I can probably test that myself. IIRC for testing WriteCalibrationParameters I needed a z probe. which I have, but not not connected or anything yet.

@tobbelobb
Copy link
Contributor

IIRC for testing WriteCalibrationParameters I needed a z probe. which I have, but not not connected or anything yet.

I think there's a manual procedure that let's you collect the same bed height mesh data that a probe would collect, it just takes a lot more time to step through.

@tobbelobb
Copy link
Contributor

I will unfortunately be away from my Hangprinter for three to four weeks. I'll try to think about ways we could test this without access to physical machine.

@tobbelobb
Copy link
Contributor

I made two commits on top of this branch. You can find them here: https://github.com/tobbelobb/RepRapFirmware/commits/3.5-dev-hangprinter-simplify

They have not been compiled and tested.

The change to StaticForces() function has been tested though, with a commit in the flex compensation repo, here: https://gitlab.com/tobben/hangprinter-flex-compensation

I didn't manage to get rid of AXIS_A completely today, but isolated it into only two functions who still use it.

@jtimon
Copy link
Contributor Author

jtimon commented Apr 9, 2023

Thank you. It looks good at a glance. More anchor treatment turned into loops. I made a single comment in one of the commits though: tobbelobb@a335f44#r108198467

Perhaps it would be better if we left those 2 commits for a different PR on top of this one?
Feel free to open it.
In the meantime I guess I can try to integrate it in #601
But I don't think it will be fixed yet since I think this is wrong.

@tobbelobb
Copy link
Contributor

I looked at the this is wrong-link. What made you change the 11 to 14 in the first place?

We need @dc42 for input on the object model.

I'm currently unaware of any currently active uses of Hangprinter's entries in the object model. We shouldn't let it slow us down if we don't have to.

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