Laundry List of small Requests

nhof

Did a bit of testing this morning on my machines.

I did not see any abnormal motion if the M208 commands are the same value. e.g. M208 X0:0. Motion is normal in this case.
I did not see any abnormal motion with M208 command depending on positive or negative values e.g. M208 X-100:-90, M208 X-500:500, M208 X-50:-50. These worked as expected.
I did see abnormal motion with M208 when min value > max value. e.g. M208 X100:0, M208 X0:-1, M208 100:-100 etc. Moves after homing in this case will result in hard to predict motion.

Motion was not that different with different values for M574 (for me). This could depend on homing scripts but I didn't bother testing any of this.

I also did not see any non-commanded moves on alternate axes (like XY moves when Z move was commanded), but I am using external drivers on drive 5, 6, and 7 for my axes so that may have an impact.

Poking around in the code, it looks like it may be an issue with the end coordinates of a move being re-calculated if the end points of a move fall outside the axis limits, as this will always be the case if the M208 command has been given inverted axis limit values and may result in the end points of moves being offset by the entire axis length in the opposite direction from the expected one.

I still think the best thing to do would be to add input validation on the M208 command. I think it is reasonable to require that the minimum limit is equal or less than the maximum limit.

Here is the file GCodes2.cpp with a little input validation. I don't have the build set up for RRF so I have not compiled or tested this code and it might need refactoring to clean it up, but the idea is there:

case 208: // Set/print maximum axis lengths. If there is an S parameter with value 1 then we set the min value, else we set the max value.
	{
		bool setMin = (gb.Seen('S') ? (gb.GetIValue() == 1) : false);
		bool seen = false;
		for (size_t axis = 0; axis < numTotalAxes; axis++)
		{
			if (gb.Seen(axisLetters[axis]))
			{
				float values[2];
				size_t numValues = 2;
				gb.GetFloatArray(values, numValues, false);
				if (numValues == 2)
				{
					if (values[0] > values[1])
					{
						reply.catf("Invalid %c axis limits, min limit cannot be greater than max limit", axisLetters[axis]);
						result = GCodeResult::error;	
					}
					else
					{
						platform.SetAxisMinimum(axis, values[0], gb.MachineState().runningM501);
						platform.SetAxisMaximum(axis, values[1], gb.MachineState().runningM501);
					}
				}
				else if (setMin)
				{	
					if (values[0] > platform.AxisMaximum(axis)
					{
						reply.catf("Invalid %c axis limits, min limit cannot be greater than max limit", axisLetters[axis]);
						result = GCodeResult::error;	
					}
					else
					{
						platform.SetAxisMinimum(axis, values[0], gb.MachineState().runningM501);
					}
				}
				else
				{
					if (values[0] < platform.AxisMinimum(axis)
					{
						reply.catf("Invalid %c axis limits, min limit cannot be greater than max limit", axisLetters[axis]);
						result = GCodeResult::error;	
					}
					else
					{
						platform.SetAxisMaximum(axis, values[0], gb.MachineState().runningM501);
					}
				}
				seen = true;
			}
		}
			if (!seen)
		{
			reply.copy("Axis limit");
			char sep = 's';
			for (size_t axis = 0; axis < numTotalAxes; axis++)
			{
				reply.catf("%c %c%.1f:%.1f", sep, axisLetters[axis], (double)platform.AxisMinimum(axis), (double)platform.AxisMaximum(axis));
				sep = ',';
			}
		}
	}
	break;

fcwilt

@nhof said in Laundry List of small Requests:

@rflulling @dc42

I still think the best thing to do would be to add input validation on the M208 command. I think it is reasonable to require that the minimum limit is equal or less than the maximum limit.

Hi,

While this sort of sanity check is doable is this a common enough problem to justify the effort?

Are there other values that should be checked?

Frederick

nhof

@fcwilt

Good question! While it is a fringe case, I think it's worth implementing a check because of the reports of axis motion on other axes than the commanded ones. (XY motion when Z move commanded, etc.)

It is ultimately the responsibility of the user to validate their config inputs, however it's not bad to prevent inputs that would break the system or cause undefined behavior.

I think unless there is both a use case and a standard for inverting axis limits and how the inversion would interact with the rest of the system, we may just want to eliminate the potential problems at the source.

I think more validation is always better to make the system more resilient against faults, with the caveat that it should not take control away from the user. A good example would be how the newer firmware versions require homing before axis movement to prevent damage from over-travel, however the user still has the ability to turn this check off with the M564 command.

Ultimately it's up to David to make the decisions of what to include/change anyway, this is just my opinion.

rflulling

@nhof said in Laundry List of small Requests:

I did see abnormal motion with M208 when min value > max value. e.g. M208 X100:0, M208 X0:-1, M208 100:-100 etc. Moves after homing in this case will result in hard to predict motion.

Thank you, for the effort. I feel like every time I responded to reply one or tried to clarify, the rabbit hole only got longer and further off track from the original issue, and from the original post. It felt like every time I explained the explanation was some how attributed to something else, making me want to forget the post all together.

I honestly just wanted to report some bad behavior as well as make a few requests that seemed reasonable additions. Thank you, @nhof , you took the time to see it for yourself and validate that yes it is there and yes it makes the other axis do unpredictable things. -If requested I would be happy to gather more data in the the possible ways it breaks the code. I had observed more than one kind of bad behavior depending on the settings. But at the core was always that same, any time I input Multiple digits on an Axis of the M208 Command. I did not test to see if this was trickle down, meaning it only effected axis listed after. I did not check to see if I only made Z a high value would X or Y be effected if they were declared first.

I will tell you that the strangest thing i saw was telling X to go home and instead it would produce a behavior something like this. G90 G1 X15 Z25. If I pressed it again it would look like this, G90 G1 X-15 Z25. The result was an odd zigzag as if X was constantly reset. This would go on until Z eventually reached the top, if I kept hitting home.

Another odd one was X would go to the end of the line, without slamming into it, then at X115 gently return at a slower speed to X0 and stop.

The worst were of course, when no mater what was pressed, all axis behaved out order, or as if linked, and no one moved in the right direction despite setting being correct.

M208 X115 S1, or M208 X-115 S1, or M208 X10 S1 was called out in all instances, where bad behavior was observed.
-Thus the 1 digit on the S flag, or command M208, any time an axis is given a value greater than 9.
M208 X115 S0, or M208 X-115 S0, or M208 X10 S0, or M208 X0 S0 seem to behave as expected and caused no issues that I know of for certain.

bot

I noticed a bug in the firmware, too! When I misconfigure my delta printer as a corexy, the X, Y, and Z towers do all sorts of crazy things! Nothing like I'd expect from a delta printer!

Another bug I found was that when I sent the hotend to Z-456 it CRASHED the head into the bed! I had set the axis minima to Z-500 because my setup is not correctly configured. BUG!!!!

fcwilt

@bot

Now, now - be kind. Folks make mistakes and learn from them - hopefully.

rflulling

@bot you are absolutely right, thats not a bug, it's a feature! I was doing it wrong, everything works perfect, it's all perfect.

rflulling

@dc42

tested M574 X2 Y1 Z1 S0 and observed no change from M574 X2 Y0 Y0 S0.

Since M574 X2 Y1 Z1 S0 is a combination output by RRF and it offer no changes in behavior, I can put this in my guide. Thank you.

fcwilt

@rflulling said in Laundry List of small Requests:

tested M574 X2 Y1 Z1 S0 and observed no change from M574 X2 Y0 Y0 S0.

How did you test this?

Frederick

rflulling

@fcwilt

By switching out the code as-is??

I was running my machine on the prior and it is currently running the later. No observable changes in behavior.

I ran and had been running test prints prior, and have made more prints after. No issues.