3.4 WIP: Allow up to 9 anchors/motors to be configured in hangprinter kinematics. #585
Conversation
There was a problem hiding this comment.
Great PR.
I suggest splitting it into
- one PR that adds axes 5-9, all the changes to print/catf functions (looping). Any form of tests all those printouts would be great. From experience, they are easy to get wrong.
- Two PRs that deal with IsReachable
- One that generalizes the current tetrahedron algorithm to one that works for any convex N-gon shape spanned by N vertices.
- One that allows effector to hang in the volume below anchors, but above ground.
| (int)spoolGearTeeth[A_AXIS], (int)spoolGearTeeth[B_AXIS], (int)spoolGearTeeth[C_AXIS], (int)spoolGearTeeth[D_AXIS], | ||
| (int)fullStepsPerMotorRev[A_AXIS], (int)fullStepsPerMotorRev[B_AXIS], (int)fullStepsPerMotorRev[C_AXIS], (int)fullStepsPerMotorRev[D_AXIS] | ||
| ); | ||
| reply.printf("Q:Buildup fac %.4f\n", (double)spoolBuildupFactor); |
There was a problem hiding this comment.
Was this code tested? I think maybe printf will overwrite the reply. I'm a bit rusty on printf/catf, but judging from just their function names I think catf() is what we want here instead of printf().
There was a problem hiding this comment.
Thank you, I was wondering about that, and the '''reply.lcatf''' parts too.
No, none of the code was tested yet. hold on, I'm going to post the testing progress in the general thread.
There was a problem hiding this comment.
How do I test this code specifically? Can you do it for me with the code in #584 as a start? that would be awesome.
First do no harm, they say. I still would like to know how to test this part though, I already have a probe, just not pluged, that will be a bunch of CAD work, which I'm excited but kind of bad at.
| ); | ||
| if (numAnchors > 4) | ||
| { | ||
| reply.lcatf("E:%.2f, %.2f, %.2f\n", |
There was a problem hiding this comment.
Is lcatf() right? Should it not be catf() ? I would have to dig a little bit to be completely sure here.
Also, I'm a bit worried that the reply might be of limited size?
There was a problem hiding this comment.
Yep, you were right about the limit size. How do we solved this? I documented it better in the main posrt, I'm going to mark this comment as solved because I think I solved the catf part.
Feel free to open a new one if I didn't solved it or properly documented it.
Better documentation always welcomed.
|
|
||
| for (size_t i = 1; reachable && i < numAnchors - 3; ++i) | ||
| { | ||
| reachable = isSameSide(anchors[i], anchors[i + 1], anchors[numAnchors - 1], anchors[i + 2], coords); |
There was a problem hiding this comment.
This line of code divides anchors into
First anchors: (i) and (i+1)
Last anchor: (numAnchors - 1)
Mid anchor: (i + 2)
It stipulates that if mid anchor is on one side of the triangle between first+last anchors, then coord must be on the same side.
This requires the user to put first anchors in either clockwise or anticlockwise order around an axis going through last anchor, I think. If I'm right then anchor placement becomes not really arbitrary anymore.
Anyhow, this function needs a big battery of tests since it's hard to follow and reson about.
If we really want a function that works for any convex reachable volume, then we need to take the set of vertices (anchors), construct all triangles that span the convex outer surface (similar to what loft() in cad programs does), and for each triangle check that coord is on the same side as the non-triangle vertices are.
There was a problem hiding this comment.
To allow effectors to travel below the spanned volume, as I know you want to, it would be useful to run a 2d-version of the algorithm first, and if coord is below all anchors (but not too far into negative z), then the point is reachable.
There was a problem hiding this comment.
To sort a set of 2d-points, and extract the convex hull of them, you can do like line-collision-detector does, here: https://gitlab.com/hangprinter/linc/-/blob/master/linc/linc.c%2B%2B#L47
There was a problem hiding this comment.
What I assumed isSameSide does:
You provide 3 points defining a plane.
You provide 2 additional points.
The plane divides space in two. if the two additional points are on the same side of that bisection of space, it returns true and otherwise it returns false.
I'm trying to be precise, but perhaps I'm losing on clarity.
But I just assumed that without even trying to understand the math inside the function, just by looking at how it was being used and because that made sense to me.
Based on that, what I'm trying to do here still assumes a pyramid configuration, like in the hangprinter.
Just allows pyramids with one extra side (one extra triangle) for each extra anchor.
This assumes the last anchor is the one on the top of the pyramid (which I think it's already an assumption with the current 4-anchors implementation).
So what this supposedly does is going through all the sides in the base (which is only a triangle for 4 anchors) and with those two points plus the point on top of the pyramid (the last anchor) you get each of the pyramid triangles.
Then for the base, any 3 points within the base will serve to define the plane.
Anyhow, this function needs a big battery of tests since it's hard to follow and reason about.
100% agreed. Can you help me with that my pointing where the unit tests are and how they are run?
I can't find them, but this PR should definitely come with some tests, I think.
Once we get all the tests for the pyramid printing space we can try to generalize further and remove the assumption of the pyramid shape and the last anchor being on top.
Probably M669 could have some validations and return some errors if certain configurations are not supported.
My plan was to next move from pyramid to prism + pyramid (pyramid being a special case where the prism on the bottom has height=0) and then also allow prism only too, without the need for a top of the pyramid anchor.
We could generalize further, to any volume without anchors in the middle, I think (even the most generalized case would need validations in M669, I think), but what I'm looking for is just a prism.
A quadrangular prism, well, rather pseudo-quadrangular, but octogonal, because I want 2 anchors on each corner to open the door for the extra 3 degrees of freedom.
Well, I shouldn't get too ahead, one step at a time.
Just optionally moving from a triangular pyramid to a quadrangular pyramid would be a really nice first step in my opinion, since most rooms are squared (or kind of) in my experience.
Like, one anchor in each corner of the room/closet kind of thing.
But, yeah, eventually I just want a squared prism as printing space, just like more traditional printers.
Sorry for the long rant.
There was a problem hiding this comment.
if the two additional points are on the same side of that bisection of space, it returns true and otherwise it returns false.
Correct.
Can you help me with that my pointing where the unit tests are and how they are run?
Sadly I don't think this repo has established much in terms of unit testing. Whenever I've implemented tricky functions, I've made separate repos for developing those functions in isolation. Eg https://gitlab.com/tobben/hangprinter-forward-transform and https://gitlab.com/tobben/hangprinter-flex-compensation.
The linc-repo has better testing structure, and proper data types for points/vertices. I suggest extracting much of what you need from that repo. Even some of the geometrical functions you need are already there. https://gitlab.com/hangprinter/linc
If you make a separate development repo and point me to it I could help you extract some of the relevant stuff from linc.
Once we get all the tests for the pyramid printing space we can try to generalize further and remove the assumption of the pyramid shape and the last anchor being on top.
Sounds like a solid plan.
|
Some clarifications on how do I plan to test this myself. A) I'm using non duet hardware for now. I build the gloomyandy branch with my changes on top, and before pushing here I rebase on top of the Duet3d branch removing all his changes. As long as I'm only touching kinematics code, I think everything that works for me should work for other boards as well, but the more boards that can be testing before merging, the better. Including duet boards, of course. B) I don't have a hangprinter and I'm not building a "proper hangprinter" (4 anchors forming a tetahedron) myself at this point. I'm building a "reprope" (8 anchors on the same plane on top). Any testing for any other number of anchors or configurations will be done without ropes/lines at all. Just looking at if the motors that should be moving are actually moving as they roughly should be moving. Call for testers: So far just I tried:
conclusion: there's something really wrong with the code, as usual with the first try. Sorry, talking too much, testing and coding too little. Damn, I hate testing until stuff works, when I love it because it's easy right when I don't need it anymore. It would be cool to have kinematic unitests. Sorry, too much thinking out loud, until next time. Keep code nits coming, please. But I don't know what tobben's longest comment brings. |
|
results with all standard hangprint and the first commit: noises and no motors moving. |
jtimon
changed the title
jtimon
force-pushed
the
3.4-hangprinter-9axes
branch
from
51446ee to
04da771
Compare
|
Your suggestion for dividing the PR is good. For now the different ideas about Isreacheable we can keep discussing here though, I will try to separate in commits, I'm already ding it. I hope the isInsideSquarePyramid function in the commit "Hangprinter: Allow Hangprinter to be configured with 5 anchors". Context: both M669 and M666 your current respective configuration for them if you call them with no arguments. I'm not sure what to do about this. Thoughts? Regarding motor testing, it seems to work with all 8 motors, without weird noises anymore and after configuring with the hangprinter. I still don't have 9 plugged motors in the same machine to test, only 8. Also, what was pointed out wasn't tested in #584 , there's no need to reiterate. |
jtimon
force-pushed
the
3.4-hangprinter-9axes
branch
from
04da771 to
da87aeb
Compare
| (double)printRadius | ||
| ); | ||
| reply.lcatf("P:Print radius: %.1f", (double)printRadius); | ||
| // TODO FIX messages are too long if there are 7 anchors or more |
There was a problem hiding this comment.
Maybe we can just shorten the format. Like drop "Print radius". And in the lcatf-loop below, use
%c:%.0f,%.0f,%.0f"
Is there a newline character missing at the end of that string by the way?
There must be some kind of trick, to write reply to console, and re-fill reply with the rest before returning but not sure how. @dc42 knows
There was a problem hiding this comment.
The way that RRF handles long responses in RRF is to pass a reference to a pointer to an output buffer as well as the StringRef to the reply buffer. That gives the function the option of allocating and output buffer and constructing the response in that instead. An example is function Platform::ConfigureStallDetection. An output buffer is only allocated if the response is likely to be too long to fit in the reply buffer, and only when it is known that the command will return success.
There was a problem hiding this comment.
I could drop the print radius, I guess, but I think @dc42 's proposed solution is superior. Having an example to copy from, it shouldn't take me that much. Sorry, I have been distracted with other things and I didn't find the time to come back to this. Once I'm done with those things, I will keep working on this, sorry.
There was a problem hiding this comment.
If I'm understanding this correctly, I would need to add a OutputBuffer *& buf parameter to HangprinterKinematics::Configure, which means I need to add it to Kinematics::Configure and thus to all other implementations of ::Configure of classes that inherit from Kinematics.
Is that correct?
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
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.
This allows to print square pyramids instead of tetrahedrons, at the cost of an extra motor.
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
jtimon
force-pushed
the
3.4-hangprinter-9axes
branch
from
da87aeb to
e5efcd6
Compare
| private: | ||
| // Basic facts about movement system | ||
| static constexpr size_t HANGPRINTER_AXES = 4; | ||
| const char* ANCHOR_CHARS = "ABCDEFHIJ"; // Skip the G, since it is reserved in G-code |
There was a problem hiding this comment.
I think we should reserve the letter E for extruder axes, and F for speed parameters. If we can, I think it would be best to reserve all the G1-related parameter letters (XYZEFHSRP), as well as G, M and N. So my suggestion for anchor chars:
const char* ANCHOR_CHARS = "ABCDIJKLO";
There was a problem hiding this comment.
It makes sense to me. Only G seemed to conflict when calling M669, which is, I think, the only place where it matters, so ABCDEFHIJ seems to work just fine. Apart from there these labels are only used internally in the C code, but never again in the gcode, I think, but I could be wrong. Or new opcodes could use them in the future or something, so it makes sense to play safe. Your proposal sounds good to me. I will change this, thanks for the suggestion.
It just came to mind that perhaps we could reserve some of the codes in M584 for additional axes too.
UVWABCD.
At some point in the future I would like to add at least 3 additional axes for my 8 anchors machine, since with my setup I should be able to get that "for free" (in terms of hardware), so I would need at least UVW, I think. To play around with non planar printing.
I thought that wasn't relevant since, as said, from my understanding, apart from G669, hangprinter anchor labels will only be used internally in the firmware, not in the gcode.
Which brings me to a question I was meaning to ask @dc42 in the future, or I guess just find the code and try to figure out for myself.
Can G1 accept UVWABCD parameters ? I was assuming yes, looking at other projects that seem to be using more than 3 axes and are using RRF too, like https://github.com/FreddieHong19/Open5x
I have many questions about using more than 3 axes, but I was waiting to research the topic a bit more myself before asking to hopefully ask smarter questions.
If we would skip UVWABCD too, the result would be IJKLOQT...mhmm, did we run out of letters?
Could we call them AN0...AN9 or something else longer than 1 letter per parameter in G669?
Sorry if the latter is a stupid question, I'm still kind of new to G-code.
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
…hors The response messages in M669 and M666 when sent empty won't fit The solution is started here: Duet3D#585 (comment)
jtimon
changed the title
This allows the hangprinter kynematics to be configured with more than 4 anchors/motors, up to 9 at a maximum.
I don't know if allowing more anchors would be possible, but I'm assuming 9 is the maximum because M584 can only map XYZUVWABC.
I personally want to use 8 anchors for my little project (https://github.com/jtimon/reprope) and perhaps nobody will ever want to
use 9 anchors to begin with, so I don't think it would make sense to go further than this, in principle.
With 4 anchors you get a tetahedron printing volume.
With 5 anchors, you get a squared pyramid printing volume.
With 6 anchors, a pentagonal pyramid.
With 7, hexagonal pyramid
etc.
This still assumes one anchor on the top and the rest bellow the bed. But I plan to create another PR exploring allowing other anchor positions and thus other printing volume shapes.
So far this is completely untested.
I separated the additions of an extra anchor/motor so that we can perhaps merge things as we test them.
Of course, assuming there's not something terribly wrong with this that I'm not aware of.
We should test:
But I can always squash as preferred or separate things differently if it is preferred.
Note: the MotorStepsToCartesian could have less error if all the anchors were used instead of just 4 of them for all cases, but that requires much more work.
#584 being merged first would make this diff a little bit smaller.
Dependencies: