Make arrays for arms #175
Open
Make arrays for arms #175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Since each commit is intended to not change anything, it could be merged at any point and the remaining work done as an additional PR, but I have it marked as a draft for now. But since it is so invasive, if other people want to work on the codebase, it may be worth merging it to avoid merge headaches in the future |
|
one to-do for this is to look at the bool arrays and see if they can/should be replaced by bitmasks. I think that will simplify some things (and eliminate some fooAll variables) but that sort of change is beyond the current focus of this PR. |
davidelang
force-pushed
the
make_arrays_for_arms
branch
2 times, most recently
from
ab8546d to
2681867
Compare
see [Projects] Modifying Maslow 4 to a cut Area of 28'x6' to cut synthetic fabric for a use case.
instead of using magic numbers (0-3) to reference the arms, create constants tl/tr/bl/br convert Maslow.cpp to use these constants instead of numbers this first commit adds a minor amount of clarity, but it's real value will show up in later commits in the series
change many variables from separate per-axis variables to an array indexed by the arm constants This is a large diff, but it is separate so that it can easily be reviewed and validated as 'obvioulsy' correct (or not)
This number is the length past where retraction stops that the belt actually extends. This can be from putting a stop on the belt so it doesn't retract all the way (useful so that you can do retraction without having to disconnect from anchors) or an added length added to the belt to be able to reach additional distances In both cases, you figure out what value to put in by retracting with and without the modification if you are adding a clamp, retract without the modification, extend, add the clamp and then retract again. The distance reported when the belt is fully retracted is the value to enter for that arm. If you are adding an extension, clamp the router in place, hook the belt to an anchor (further away than your extension), retract until it stops, extend, hook up your extension, retract again. The absolute value of the distance reported is what you enter for that arm.
This should not make any difference unless there is something buried somewhere that is doing calculations based on the old order but not being sure how the dats is passed to the UI for calibration this may be the case.
davidelang
force-pushed
the
make_arrays_for_arms
branch
from
2681867 to
e7031a4
Compare
|
This one is ready to merge? |
|
BarbourSmith wrote:
This one is ready to merge?
Yes, this should be good.
David Lang
|
BarbourSmith
reviewed
Dec 4, 2024
FluidNC/src/ProcessSettings.cpp
Outdated
| @@ -833,31 +833,31 @@ static Error maslow_retract_TL(const char* value, WebUI::AuthenticationLevel aut | |||
| return Error::ConfigurationInvalid; | |||
| } | |||
| sys.set_state(State::Homing); | |||
| Maslow.retractTL(); | |||
| Maslow.retract(tl); | |||
There was a problem hiding this comment.
I'm getting an issue that tl is not defined in this scope. It seems like it should be since Maslow.h is included, but this is preventing this from building for me
|
BarbourSmith wrote:
> @@ -833,31 +833,31 @@ static Error maslow_retract_TL(const char* value, WebUI::AuthenticationLevel aut
return Error::ConfigurationInvalid;
}
sys.set_state(State::Homing);
- Maslow.retractTL();
+ Maslow.retract(tl);
I'm getting an issue that tl is not defined in this scope. It seems like it should be since Maslow.h is included, but this is preventing this from building for me
the fix for this is change it to
+ Maslow.retract(Maslow.tl);
I know I compiled this in testing, so I don't know what happened.
do you want a new commit with this fix? or do you just want to change the 4
lines?
the diff would be:
diff --git a/FluidNC/src/ProcessSettings.cpp b/FluidNC/src/ProcessSettings.cpp
index 524fa216..beaa9ce4 100644
--- a/FluidNC/src/ProcessSettings.cpp
+++ b/FluidNC/src/ProcessSettings.cpp
@@ -833,7 +833,7 @@ static Error maslow_retract_TL(const char* value, WebUI::AuthenticationLevel aut
return Error::ConfigurationInvalid;
}
sys.set_state(State::Homing);
- Maslow.retract(tl);
+ Maslow.retract(Maslow.tl);
return Error::Ok;
}
static Error maslow_retract_TR(const char* value, WebUI::AuthenticationLevel auth_level, Channel& out) {
@@ -841,7 +841,7 @@ static Error maslow_retract_TR(const char* value,
WebUI::AuthenticationLevel aut
return Error::ConfigurationInvalid;
}
sys.set_state(State::Homing);
- Maslow.retract(tr);
+ Maslow.retract(Maslow.tr);
return Error::Ok;
}
static Error maslow_retract_BR(const char* value, WebUI::AuthenticationLevel auth_level, Channel& out) {
@@ -849,7 +849,7 @@ static Error maslow_retract_BR(const char* value,
WebUI::AuthenticationLevel aut
return Error::ConfigurationInvalid;
}
sys.set_state(State::Homing);
- Maslow.retract(br);
+ Maslow.retract(Maslow.br);
return Error::Ok;
}
static Error maslow_retract_BL(const char* value, WebUI::AuthenticationLevel auth_level, Channel& out) {
@@ -857,7 +857,7 @@ static Error maslow_retract_BL(const char* value,
WebUI::AuthenticationLevel aut
return Error::ConfigurationInvalid;
}
sys.set_state(State::Homing);
- Maslow.retract(bl);
+ Maslow.retract(Maslow.bl);
return Error::Ok;
}
static Error maslow_retract_ALL(const char* value, WebUI::AuthenticationLevel auth_level, Channel& out) {
|
|
I shoulda seen that. I can easily make that change. Testing now. |
|
When I go to run calibration on this PR it spools out the belt endlessly, have you tried calibration on it? |
|
BarbourSmith wrote:
When I go to run calibration on this PR it spools out the belt endlessly, have
you tried calibration on it?
No, I do not yet have the ability to put this on the real hardware, let alone
run it.
As I noted earlier, this is a big pile of changes, it may be best to add the
commits a few at a time and test them.
David Lang
|
|
Gotcha, I'll try to find time to do that, but it might take me a bit |
|
It's a lot easier for me to merge things if they've been tested at least a bit on hardware |
|
BarbourSmith wrote:
Gotcha, I'll try to find time to do that, but it might take me a bit
If you look through the commits one at a time, I believe that one of them added
a huge amount of changes to the calibration routines.
I did buy an extra set of electronics, and motors just arrived last night, so I
need to solder them in place and dig up a couple of steppers, then I should be
able to put it on the real hardware to see it boot and pass the tests. not as
good as testing it with belts on a real frame, but getting a step closer
David Lang
|
|
checking on this what were you able to merge? |
|
By far the hardest part of programming isn’t writing the code, it’s writing the code that works and fixing it when it doesn’t. 90% of the work is in the testing and debugging, and it’s especially hard to try to debug someone else’s code which has issues. If you can get this code to work and test it on a machine I’m happy to merge it in (it’s a great idea), but if it doesn’t work it’s probably going to be faster for me to close this PR and write it from scratch testing along the way. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a major effort to change from many individual variables to arrays. By the end of it, many separate functions will be collapsed, reducing code duplication. It is a work in progress, but each commit is designed to stand alone as an improvement over the prior state, without changing anything that would be visible to the user or the UI code.