New source of error

If you opening up the yaml file could re-enable the Z default heights so we don’t have to manually edit file to set them please.

1 Like

Ian Abbott wrote:

If you opening up the yaml file could re-enable the Z default heights so we
don’t have to manually edit file to set them please.

Everything in the yaml file can be set under the advanced setting in the fluidnc
page where you upload the yamnl file (a different tab, but same popup window) so
even if you are setting things, you should not have to manually edit the yaml
file to do so.

David Lang

2 Likes

Let’s do one change at a time so we know which change lead to an improvement (or issue)

Bar wrote:

Let’s do one change at a time so we know which change lead to an improvement (or issue)

makes sense, do you want me to submit a PR for index.html to remove the
project() function to go with this change? or will you do that?

(we either should remove the project() function and use the second commit in my
PR, or modify the project() function to use the correct math, I think removing
it is better)

David Lang

1 Like

It’s tricky to make changes that touch so many places at once. The easiest option is to make a PR to to the index.html and to the firmware at the same time which have been tested together.

Are you able to test them atm? I just want to make sure that we aren’t regressing (it’s easy to do)

Bar wrote:

It’s tricky to make changes that touch so many places at once. The easiest option is to make a PR to to the index.html and to the firmware at the same time which have been tested together.

aren’t they in separate repos? afaik a PR can only be in one repo.

Are you able to test them atm? I just want to make sure that we aren’t regressing (it’s easy to do)

No I am not.

David Lang

1 Like

Where is this function used? I see it used in projectMeasurements() right after it in the file, but I don’t see anywhere that projectMeasurements() is used

Also, this may make the scaling correction that you were putting in incorrect/unneeded, I would back that out when first testing with the new math (the scaling is a fudge factor, this may eliminate the need for it, or make it need to be higher)

1 Like

Interested to try this once merged. Thanks for pushing on the accuracy @dlang

1 Like

Yeah, but if you make two PRs they can reference eachother in the description making it clear that they need to be tested together and that merging one without the other will mess with calibration accuracy because the angles won’t be projected.

This is top of my list for today so hopefully we’ll make some progress this week. Back in Seattle and able to test things

1 Like

Bar wrote:

Yeah, but if you make two PRs they can reference eachother in the description making it clear that they need to be tested together and that merging one without the other will mess with calibration accuracy because the angles won’t be projected.

This is top of my list for today so hopefully we’ll make some progress this week. Back in Seattle and able to test things

Thanks, that’s what I was thinking of, but I couldn’t find where the
projectMeasurements fucntion was called. eagerly looking forward to this.

David Lang

1 Like

You were correct, it was not being called anywhere :man_facepalming:, I just made a PR to fix that. Great catch!

The next thing on my todo list is to tackle the new gradient decent algorithm which should help with finding more precise anchor locations and then add the new math for including the arm angle.

I’d like to do the first one before the Wednesday update and the second one after because I want to take the time and test carefully so that we can be sure which changes are making things better. I don’t want to end up in a situation where we change too many things at once and then it’s not clear which ones helped and in what ways.

1 Like

Bar wrote:

You were correct, it was not being called anywhere :man_facepalming:, I just made a PR to fix that. Great catch!

the second commit in my PR should elimninate the need to have it in the UI by
correcting for Z in the firmware and only passing the projected values over to
the UI for calibration.

But this could explain some of our calibration struggles.

David Lang

Bar wrote:

The next thing on my todo list is to tackle the new gradient decent algorithm which should help with finding more precise anchor locations and then add the new math for including the arm angle.

I’m not placing the term, what is the gradient decent algorithm?

I’d like to do the first one before the Wednesday update and the second one after because I want to take the time and test carefully so that we can be sure which changes are making things better. I don’t want to end up in a situation where we change too many things at once and then it’s not clear which ones helped and in what ways.

That makes sense, even just doing the projection in the UI like you thought you
were doing would probably make a significant difference, but if you’re adding my
fix, I don’t know that it’s worth breaking it down in two steps in released code
(I could see you wanting to test each step by itself, but I don’t think it’s
reasonable to push that out to the public)

David Lang

1 Like

The one discussed here: Loose belts during calibration - #67 by bar

It seems to be somewhat unstable though which is giving me pause

Bar wrote:

The one discussed here: Loose belts during calibration - #67 by bar

It seems to be somewhat unstable though which is giving me pause

can it be added as an option for people to try?

But I think we want to fix the projected length → belt length and belt length
→ projected length calculations before we do anything else.

David Lang

1 Like

I thinking through what the best way to do it is and I made a diagram to clarify things:

I think that you are right that the right place to do the projection is in the firmware. The firmware has easier access to things like settings so if we want to expose some of these options as user settings having it in the firmware would be the right place to do it.

do you want me to update the PR to remove the z that isn’t defined in the projection? or will you fix it?

1 Like

I’m on it, testing things now

1 Like