MaslowCNC / Firmware

Maslow Firmware
GNU General Public License v3.0
261 stars 136 forks source link

'TestMotors' bug for TLE5206 boards #500

Closed blurfl closed 5 years ago

blurfl commented 5 years ago

Somewhere after the ~v1.25~ v1.26 release one of the changes has caused TLE5206 boards to fail the Axis::test() function. Stock Maslow boards are not affected. The release v1.26 runs correctly. When the test is run, each motor runs and passes Direction 1 but does not run in Direction 2.

blurfl commented 5 years ago

git bisect finds that PR #486 is the last version where the motor tests worked correctly, PR #489 marks the presence of an issue.

Digging deeper, this commit seems to be related to the problem. Before the change in this commit, the motor test runs normally on the stock Maslow v1.2 board as well as the TLE5206. After this change to the repo as it stood at that point, both boards fail to run properly but in a manner different from the behavior described in the initial issue. I've tried backing out the later changes to Axis::test() and Motor:::detach() which seemed likely possibilities, no joy. I'll keep digging, suggestions welcome.

BarbourSmith commented 5 years ago

Hmmm it's interesting that an issue like that would be caused by writing to the eeprom. I wouldnt expect that behavior.

I think this issue brings up a larger question about if there is a way to do hardware testing automatically on pull requests before we merge them. I will look into building some sort of rpi Arduino rig that will put some hardware through it's paces and comment on pull requests if all tests pass.

blurfl commented 5 years ago

I don't think it has anything to do with writing to the EEPROM, but at that point in the change commit sequence the frequent writes affected something in the test timing that caused all boards to fail the tests. After PR#495, only the TLE5206 boards fail, and they only fail the second half of each motor test. If you swap the forward and reverse cases within the test, it is still the second case that fails. And to make matters stranger, there's this: the two test cases are

motorGearboxEncoder.motor.directWrite(255);

and run for a while and check that the encoder counts went the direction expected then

motorGearboxEncoder.motor.directWrite(-255);

and again check the encoder counts.

If you change the second case to directWrite(-254) instead of -255 (or 254 if swapping the sequence), the TLE5206 will pass all the motor tests. But why? There has to be something in the TLE5206 section of Motor::write() but I haven't trapped it yet. Ideas welcome...

BarbourSmith commented 5 years ago

hmmm I do have a thought on that! I believe that the pwm library handles 255 as a special case and toggles the pin high instead of pulsing it. How that is related I'm not sure, but that feels like a connection to me.

blurfl commented 5 years ago

That's a good lead, I'll try to find out about it. Thanks.

BarbourSmith commented 5 years ago

I guess the big question is what is different between the boards. I know that different pins are connected to different internal timers and that could be the connection.

esspe2 commented 5 years ago

(edited)Interesting... I'm using L298 modules and my B04 isn't working as expected either. (While GC seems happy and does the requested moves when jogging and with the test gcode file.) At least I see many spikes on the pwm outputs so the motors move someway, but if I move the directWrite(255) back outside the loop, the remaining spike is barely visible. And no 490Hz.

That behavior has appeared and disappeared several times through the firmware uploads and the wipes ( $, #, *), but I didn't note which made that. I'm also looking around the PID for any interaction or conflict between pin changes.

Edit:

Yes, I forgot that: at some point, following your advice, I reverted back to this:

if (!leftAxis.attached() && !rightAxis.attached() && !zAxis.attached()){ if 

Instead of that:

(sys.writeStepsToEEPROM && !leftAxis.attached() && !rightAxis.attached() && !zAxis.attached()){

Which seems a good starting point; I'll confirm that soon if you like

esspe2 commented 5 years ago

Hope this helps, here are some measures made on a pwm pin during the B04 test.

With if (sys.writeStepsToEEPROM test in System.cpp, the motor tests fail:

Without it, the motor tests pass:

So I'll live without if (sys.writeStepsToEEPROM for now, and at least we know that EEPROM.put() takes care of the EEPROM.

To be continued...

blurfl commented 5 years ago

At least I see many spikes on the pwm outputs so the motors move someway, but if I move the directWrite(255) back outside the loop, the remaining spike is barely visible.

This is watching one of the _pwmPins with a scope while running the B04, yes? Are you using one of the stock Maslow boards? Which board revision? I'd like to take a look at it here.

esspe2 commented 5 years ago

No, still a prototype: just 2 L298 modules and a bunch of wires, with pins VERS1 and VERS2 connected to 5V. So I can move any pin where I want. V1.2 displayed by the firmware.

davidelang commented 5 years ago

On Wed, 22 May 2019, esspe2 wrote:

No, still a prototype: just bare bones, 2 L298 modules and a bunch of wires, with pins VERS1 and VERS2 connected to 5V. So I can move any pin where I want.

I would suggest trying to find an existing board that has the same pin functionality and match it. Eventually we will run out of board IDs, so we only want to use a new one when there is an incompatibility with the existing boards, or a system problem we need to fix (for example, finding that an existing pwm pin wasn't as reliable and so needing to move it)

David Lang

blurfl commented 5 years ago

This issue was addressed in PR #501.