Motion Controll seems to be way too Simple

[KarlZeilhofer]

I just reviewed some parts of the code.

the 'move()' function is a bit too simplistic:

void move(int _idler, int _selector, int _pulley)
{
	int _acc = 50;

	// gets steps to be done and set direction
	_idler = set_idler_direction(_idler); 
	_selector = set_selector_direction(_selector);
	_pulley = set_pulley_direction(_pulley);
	

	do
	{
		if (_idler > 0) { PORTD |= 0x40; }
		if (_selector > 0) { PORTD |= 0x10;}
		if (_pulley > 0) { PORTB |= 0x10; }
		asm("nop");
		if (_idler > 0) { PORTD &= ~0x40; _idler--; delayMicroseconds(1000); }
		if (_selector > 0) { PORTD &= ~0x10; _selector--;  delayMicroseconds(800); }
		if (_pulley > 0) { PORTB &= ~0x10; _pulley--;  delayMicroseconds(700); }
		asm("nop");

		if (_acc > 0) { delayMicroseconds(_acc*10); _acc = _acc - 1; }; // super pseudo acceleration control

	} while (_selector != 0 || _idler != 0 || _pulley != 0);
}

A simple but better approximation could be like this:

int delay = 50000;

while(acelerating){
  ...
  sleep_us(delay + minDelay);
  if(delay > 20){
    delay = (delay/4)*3; // which could be even more efficently witten with d2 = d/2; d = d2 + d2/2;
  }else{
    delay = 0; 
  }
}

Here an 1/x function is approximated by an a^(-x) function, which is simpler to calculate, since no division is needed.

Starts with 66% of end speed

Quick calculations in Libre Office show, that the speed for e.g. the idler is "ramped up" from 666 steps/s to 1000 steps/s. That's far away from a proper acceleration. It starts with 66% of the end speed immediately.

No Deceleration

But the larger problem seems to be, that there is absolutely no deceleration implemented!

Inconsistent Speeds

If the function is called with multiple non-zero parameters, the speed steps, when one of them finished it's movement!

[KarlZeilhofer]

PS

Ignoring Stall Guard

The move() function also ignores the Trinamic stall guard completely! Man, you have a state of the art high performance stepper driver, but ignoring it's very basic features!

[KarlZeilhofer]

Same applies to load_filament_withSensor()

	float _speed = 4500;
	const uint16_t steps = BowdenLength::get();

		for (uint16_t i = 0; i < steps; i++)
		{
			do_pulley_step();

			if (i > 10 && i < 4000 && _speed > 650) _speed = _speed - 4;
			if (i > 100 && i < 4000 && _speed > 650) _speed = _speed - 1;
			if (i > 8000 && _speed < 3000) _speed = _speed + 2;
			delayMicroseconds(_speed);
		}
	}

Further more here a float is used, even though it could be a far more efficient uint16_t.

[KarlZeilhofer]

With this crap of motion algorithms, spread around the whole code, I would not suggest to call the version to be released anything with 1.x.x! Especially with all the bug reports about misbehaviour of the selector and idler.

[AbeFM]

I'm glad Karl took the effort to dig into the code.

I've seen LOTS of losing track of where you are, etc. When the printer says to go to a channel, the MMU moves (some parts) left or right one, but it has no idea where it IS. I understand there aren't sensors on everything, but calling positions explicitly seems like it could avoid a lot of this "four left and 3 right" - that's how everything gets out of sync.

[KarlZeilhofer]

I've fixed that issue in a simple showcase:
https://shop.prusa3d.com/forum/general-discussion-announcements-and-releases-f53/improvements-to-the-mmu-2-0-controller-firmware-t25189.html

[awigen]

I have opened a pull request using the AccelStepper library:
#66
This cleans up a lot but there is still a lot more to do.

[AbeFM]

I've played with Karl's current firmware a bit and can at least confirm it is much much faster and gets lost a lot less, has stallguard basically working and some other bits. It has a totally different feel and brings a hint of a smile to your face.