3.5 Hangprinter: Generalize IsReachable() to any pseudo-pyramid #598
Conversation
| bool HangprinterKinematics::IsReachable(float axesCoords[MaxAxes], AxesBitmap axes) const noexcept /*override*/ | ||
| { | ||
| float const coords[3] = {axesCoords[X_AXIS], axesCoords[Y_AXIS], axesCoords[Z_AXIS]}; | ||
| return isInsideTetrahedron(coords, anchors); | ||
|
|
||
| bool reachable = isSameSide(anchors[0], anchors[1], anchors[2], anchors[HANGPRINTER_AXES - 1], coords); |
There was a problem hiding this comment.
The function seems to assume that the last anchor is the high anchor. All other anchors are low anchors.
This first test makes sure coords is above triangle spanned by first three anchors.
In the 1h4l case, the triangles [second, third, fourth], [third, fourth, first] and [fourth, first, second] were not checked although they probably should be for consistency. All triplets of neighboring anchors should be checked if one of them are.
Another comment, if all bottom triangles get checked: If the fourth anchor in the 1h4l case was a few mm higher than the other three, then the position of the first anchor would become non-reachable with the given algorithm. It disallows any line coming from a low anchor to go below horizontal I think, forcing the mover to always be constrained in all the directions. This might be what we want but it's also kinda restrictive. Included such a test case in the godbolt tests.
A more elegant and general but also less restrictive that would be welcome. But checking all bottom triangles is ok for now I think.
There was a problem hiding this comment.
The function seems to assume that the last anchor is the high anchor. All other anchors are low anchors.
This first test makes sure coords is above triangle spanned by first three anchors.
This is all correct.
Here we're checking that the point in coords is above the plane defined by the base of the pyramid. If the top anchor and coords are on the same side of the space bisected be the plane defined by the base of the pyramid, then it is reachable.
In the case of triangular pyramids, the plane of the base can obviously be defined by anchors 0, 1 and 2, because it's all the vertices we have (this line literally replaces the first line in isInsideTetrahedron for the triangular pyramid case).
Since 3 points is always enough to define a plane, this should work too for pyramids with any number of vertices, assuming all the vertices of the base are in a common plane.
It doesn't need to be a plane perpendicular to the floor or the printing bed, but this assumes they're all on the same plane.
It would be nice to verify these preconditions on configuration, but I haven't done that yet, sorry. I should definitely start by documenting this better with some comments.
I guess this approach is more restrictive, but I also find it simpler. And easier to document and verify the prerequisite. Perhaps a later PR could modify the code to make it more general.
But this way we could relatively easy expand from the basic triangular pyramid case to optionally admitting quadrangular or rhomboid pyramids too.
In practice, I only expect people trying a quadrangular setup. Perhaps putting one base anchor in each of the lower corners (or two of them in the walls half of the room instead of the corners, or something like that).
There was a problem hiding this comment.
Maybe we could even ignore the bottom triangle, and just check that coords[Z_AXIS] > z_min, where z_min is configurable through some gcode. I think such a minimum value for z already exists but I don't remember which gcode that configures it. Maybe we could find it in one of the other kinematics' implementations of IsReachable().
This would make the function work in the case where there are no low anchors as well.
There was a problem hiding this comment.
The gcode I was thinking about was M208: Set axis max/min travel. The M208 limits are checked elsewhere it looks like, so I think we can just ignore the bottom triangles between anchors.
There was a problem hiding this comment.
I guess we should check z_min here too if everyone else is doing it. Independently of this PR.
The printing bed may be above the Z of the lowest base anchor.
The way I understand it, M208 looks more like a safety feature.
You should be able to set z_min above the real z_min of your machine temporarily as a safety feature.
For example, you could, for example, want to set it because for whatever reason, temporarily, you want to print directly on an object that you've placed on top of your usual printing bed, and you call M208 because you don't want the head to go bellow that object under any circumstances.
I can see how that could be extremely useful for CNC machines, for example. But for 3d printers too if they can mechanically break things if trying to go beyond certain points.
I mean, I'm just guessing looking at https://duet3d.dozuki.com/Wiki/M208
But the way I see it, and specially if everyone else is using it except for the hangprinter, this is a bug in the hangprinter implementation of the kinematics. An easy one to solve though:
Just a commit before this one (which can be separated into a different PR) that checks for the M208 limits if they are set. Perhaps M669 in the hangprinter should set some defaults calculated from the anchors positions'.
I mean, if I remember correctly from example coreXY or carthesian configs, other printers would configure that instead of even calling M669. For all axes, not just for Z.
And even though it would be redundant for most hangprinter configs to call M208 after calling M669, it could still make sense in some cases, in my opinion.
Basically M208 is not supported for the hangprinter right now and it should, IMO.
Perhaps in addition to this (after adding some comments), in M669 we could do:
- Verify that the last anchor's Z is equal or greater than all other anchor's Z, return error otherwise
- Verify there's a plane that contains all base anchors, return an error otherwise.
- Set some M208 defaults, document that M208 must be always called after M669 for the hangprinter.
Mhmm...I don't like this solution either...
This reminds me of the printing radius we're inhering from a common base from delta printers.
I guess makes more sense for the typical printers with quadrangular prism printing volumes.
Perhaps the hangprinter shouldn't inherit from the delta printers anymore (I haven't checked). But the printing radius is not used in isReacheable either, just like any call to M208.
Perhaps there's a gcode to reset the printing radius for delta printers...no, wait, right, that's M669 too, it makes sense to me.
I guess my point is...
A) Most just do M208 in all axes. They can easily overwrite those for extra safety or special cases,
B) delta printers just do M669 and set the radius and min and max Z, I presume. But they can't reset the radius for extra safety, just the Z. Delta printers are just restricted to have their 3 "anchors" at the same distance towards the center, 0,0.
C) The hangprinter is not really using the printing radius nor M208 limits. It just calculates its own triangular pyramid instead of a circular prism like deltas. So the hangprinter doesn't really need M208 or a printing radius at all in terms of isRecheable. But it would be nice to be able to further restrict Z on purpose like I'm presuming delta's can do. But we could do for all axes, not just Z. Because the base anchors for a hangprinter don't need to be at the same distance towards the center like deltas.
Apart from that, I would like to have M208 support for my own reprope project, 8 anchors on top, but really just 4 corners, there's just two anchors per corner instead of one. But I'd rather not allow the head to go near the corners, because with my setup, with experimental heads, I could have collisions the octogon defined in M669 may not protect me from. I could have stop limits for homing in a "desktop hangprinter" and those can be more restrictive than the positions of the anchors, to take into account the side of the rotating anchors wheels and stuff.
I also want a z_max dynamic configuration by using the "motor is not moving despite your clear orders" detector in TMC2209 motors, which you mentioned somewhere for initial hoping that I can't find.
There was a problem hiding this comment.
What I should do more is work on the homing file to...
-
tense all the ropes first automatically without any manual handling of the stepper motors. I read you mentioning this somewhere in your documentation. If you could remind me where, that would be awesome.
-
dynamically callibrate z_max by pulling from all 8 ropes until they stop the same way.
-
dynamically callibrate z_min with some probe
-
dynamically callibrate x or -x, or -y or y, it doesn't matter with a limit sensor. (it can be just a click when one of the rotating wheels on that side of the "square"/"octogon" rotate too much towards their outside limit). Then go all the way back to your initial position in that axis. If you already measured both in that axis, go to the middle of it
-
do the same for -x, y and -y (apart from x) going to 4 in every iteration
-
hopefully you have dynamically callibrated all your first 3 main axes dynamically and accurately. But you didn't, because your head wasn't properly oriented. Hopefully a gyroscope can help with this. But that's a problem the pyramid config doesn't have. You know where your top is: it's the last anchor on top of the pyramid.
So, yeah, probably there should be a 1.1 step with a gyroscope.
But first do the resonance thing if you have a gyroscope. Not before 1.1, but just before you loop again from the start for smarter re-calibration.
Until error<x || loops>y
Is it not? I thought it was. I think we should just set M208 S1 Z-10 in the default config.g file in the Hangprinter repo. I think it works out of the box. Didn't look too closely in the source code but I think theres something called |
|
The function is called LimitPosition. The default implementation in the root class Kinematics just applies the M208 limits. Kinematic classes such as HangprinterKinematics can override this, and if they do then they have the option of also calling Kinematics::LimitPosition to apply the M208 limits as well as whatever else they do. The current implementation of LimitPosition for Hangprinter in our 3.5-dev branch applies the M208 Z limit and the bed radius limit.. |
jtimon
force-pushed
the
3.5-hp-any-pyramid
branch
from
76468dc
to
0e208a8
Compare
|
Pushed a better version with the feedback, hopefully. Thanks @dc42 for the clarification. The way I understand it, there was no bug like I claimed there could be, because min_z is checked in an ancestor class, in this case, a radial printer in common with delta printer. But I think that's due to having the top anchor on top. Then it makes lot of sense that that's the center and the home position in x and y. But we can also do a plane on top with no last anchor on top of the pyramid. Just a prism instead. But to keep things clean, I think that should be in another PR. |
jtimon
changed the title
|
Copy of a comment that was posted on #601 but should have been posted here: I agree that this PR allows more configurations. But I think this PR is a good opportunity to remove the base plane check even though there was a base plane check in the previous version of IsReachable(). The base plane concept was awkward from before and trying to generalize it is even more awkward. The base plane check was there just to save users from destroying hardware if they did a typo like G1 Z-1000 or similar. Since I now understand that the z_min limit would save those users anyways I think we should drop the base plane check in IsReachable(). I'm not really sure there will be much interest in supporting pseudo-pyramids with not all the anchors in the same plane, to be honest. In practice all 1h4l-Hangprinters will be pseudo-pyramids with not all anchors in the same plane, since placing four anchors in a flat plane is hard to do (even getting within 10 mm is sometimes hard in my experience), and adds no value. There is some interest in pseudo-pyramids and non-flat-bottomed pyramids, a few users including myself have used non-flat pyramids a few times already. One example use case is when I move anchors mid-print while printing very tall objects, I tend to move one anchor at a time to reduce risk, giving very non-horizontal intermediary ABC planes that must work for these super-tall objects to be printable. But my main reason for not wanting the base plane concept (although I introduced it) is the effort it would take to explain users that ABC spans a plane, not BCD, CDA, or DAB, and that moving below this plane is disallowed in firmware (why?). Sorry for asking you to remove the base plane check that I added long ago, but I didn't realize how misplaced it was before I tried to generalize it while I was reviewing this PR. If you prefer, I can wedge a commit where I remove the old base plane check before this commit, or I can remove it in a separate commit after this one. |
|
Oh, ok. I thought you wanted me to write more code for those weird cases, but removing a check is indeed easy. The only problem I see is that it may return true when it is actually not reachable if low enough bellow the lower anchors, because the code actually checks for an infinite pyramid this way. But that can be limited with the printing radius and the min_z, so I guess it isn't really much of a concern. It is like really an infinite prism with a pyramid on top what can be printed, not an infinite pyramid. So, yes, I will remove the base check, now I understand what you meant. |
jtimon
force-pushed
the
3.5-hp-any-pyramid
branch
from
0e208a8
to
adcc980
Compare
|
I added two commits:
The second commit can be left for another PR since it's kind of an orthogonal thing. Plus it's a WIP in the config sense. Please give one more round of feedback before pushing some cleaned up version and start to ask people to test it. |
jtimon
force-pushed
the
3.5-hp-any-pyramid
branch
from
adcc980
to
666896a
Compare
This is a clean generalization from 4 to N. Pseudo-pyramid because the vertices in the base of the pseudo-pyramid don't need to be on the same plane. There's no base of the pyramid as such. Better documented and cleaner thanks to @tobbelobb on github.
jtimon
force-pushed
the
3.5-hp-any-pyramid
branch
from
666896a
to
bc0ab4d
Compare
|
I squashed the commits for this PR and left the work in progress prism part for another PR: |
jtimon
changed the title
|
It is useful for any pseudo-pyramid if the bed is below any of the base
anchors.
I guess I can provide some examples if it isn't clear enough.
The code for when there's no middle top anchor I left for another PR indeed.
…On Mon, Mar 6, 2023, 18:45 Torbjørn Ludvigsen ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/Movement/Kinematics/HangprinterKinematics.cpp
<#598 (comment)>
:
> bool HangprinterKinematics::IsReachable(float axesCoords[MaxAxes], AxesBitmap axes) const noexcept /*override*/
{
float const coords[3] = {axesCoords[X_AXIS], axesCoords[Y_AXIS], axesCoords[Z_AXIS]};
- return isInsideTetrahedron(coords, anchors);
+ bool reachable = true;
+
+ // Check all the planes defined by triangle sides in the pyramid
+ for (size_t i = 0; reachable && i < HANGPRINTER_AXES - 1; ++i) {
+ reachable = reachable && isSameSide(anchors[i], anchors[(i+1) % (HANGPRINTER_AXES - 1)], anchors[HANGPRINTER_AXES - 1], anchors[(i+2) % (HANGPRINTER_AXES - 1)], coords);
+ }
+
+ // For each side of the base, check the plane formed by side and another point bellow them in z.
+ for (size_t i = 0; reachable && i < HANGPRINTER_AXES - 1; ++i) {
This check looks useful for the case when all anchors are above the
effector.
I don't think we should run it in the pseudo-pyramid case or in this PR.
This code should be introduced in a separate PR.
That PR should also introduce a config variable for choosing between
pseudo-pyramid or prism shaped checks.
—
Reply to this email directly, view it on GitHub
<#598 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHWGSXD7OQ66CBACFOZSYDW2YWENANCNFSM6AAAAAAUKWXAA4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Yeah, that's true. Makes me a bit unsure. If What to you think @dc42 ? Will the ~100 extra floating point operations per call to |
|
I changed my mind. I should worry less about the clock cycles. Running both loops in the pyramid case is probably the better option. |
|
For full compatibility with those pseudo pyramids we also need to adjust the forward transform function. Here's a helper repo for the lucky individual who gets to do that: https://gitlab.com/tobben/hangprinter-forward-transform Don't let it stop this PR though, since forward transforms are very rarely used. To my knowledge:
|
|
Thank you very much for the helper repo. I would say this one is pretty much ready too, but as usual more testing can never hurt. This is barely tested and in weird conditions only. More testing, but I think it's kind of ready. |
Perhaps this could be proven mathematically.
Unfortunately I'm not a mathematician.
4 anchors: Triangular Pyramid (aka tetrahedron)
5 anchors: quadrangular pyramid
6 anchors: pentagonal pyramid
7 anchors: hexagonal pyramid
8 anchors: heptagonal pyramid
9 anchors: octagonal pyramid
If my conjecture is wrong I want to know though.