Stop Command Causes Position Change; Likely Leading to Loss of Calibration

[krkeegan]

We have a bit of a bug.

In GC, if you press stop, you will see that the sled position adjusts just slightly, if you press stop many many times, you can move the sled position quite far. If you startup GC and press stop before any movements it seems to have a much larger effect, whereas if you have moved around the number bounce around a bit, but change much slower than if done before any movements.

This is almost certainly leading to a loss of calibration over time.

Not sure what the cause of this is, this could be a floating point error?

[krkeegan]

This seems to only be an issue in #384

[krkeegan]

Soooo, I hope this is a result of my test environment. I discovered that it doesn't seem to matter what I send, but sending anything causes the encoder steps to change. This doesn't seem unique to #384.

My test environment is nothing more than an arduino and the Fake_Servo option. What I hope is happening is that the lack of a motor shield or encoders is allowing for some spurious signal to appear on the encoder pins coming from the serial RX.

I should make it back home later tonight and I can test this out on a real machine and see if I see the same behavior. Hopefully I won't.

[blurfl]

Can you ground the encoder pins with a piece of paper clip?

[krkeegan]

Good news, this doesn't happen on a real machine.

Closing this issue, but we should consider whether to make any corrections to the fake_servo

[BarbourSmith]

I think @blurfl was right to suggest that the issue is probably from the pins floating with nothing attached.

Using the internal input pull-up resistors would probably fix it:

https://www.arduino.cc/reference/en/language/functions/digital-io/pinmode/

[krkeegan]

ooh, that could be a good addition to the fake_Servo option, i will look into that.

[blurfl]

Is there a reason not to use INPUT_PULLUP mode for all the ENC pins all the time?

[BarbourSmith]

I can't think of one. The signal from the encoders is quite low impedance so it shouldn't be a problem on that side.

I'd say let's test it carefully, but I can't think of a reason not to use INPUT_PULLUP all the time 👍

[blurfl]

While we're at it, the pins used to determine the board version could use this as well. If they were all pulled up internally, the PCB would only need to ground the ones that need to be low. For future versions...

[BarbourSmith]

You know if we wanted to be really slick we could do pullups on all the pins for detecting board version and then declare the 1111 version number to be no board attached and switch to the simulator mode

On Feb 8, 2018 10:41 AM, "Scott Smith" notifications@github.com wrote:

While we're at it, the pins used to determine the board version could use this as well. If they were all pulled up internally, the PCB would only need to ground the ones that need to be low. For future versions...

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/MaslowCNC/Firmware/issues/387#issuecomment-364208451, or mute the thread https://github.com/notifications/unsubscribe-auth/AI7QV3vFzV6J4R-3qjq0NEeu7Tujfgxlks5tSz_YgaJpZM4R7_ho .

[krkeegan]

That is starting to sound a bit too polished for us :stuck_out_tongue:

We just need to be careful that this doesn't get tripped during regular operation. I think the default maybe should be to warn the user (maybe the shield fell out or broke) and somehow force some affirmative act to enable the simulation.

[blurfl]

I'd rather see a setting on the Advanced page for simulator mode, and something visible to indicate that mode (change the red cursor to blue?) It might be one that isn't saved to groundcontrol.ini so that it doesn't persist. I was recently using simulator mode to test software for a different powercontrol board while I waited for the boards, and forgot to turn it off when I began testing the boards themselves 🙃.

[blurfl]

As far as the encoders go, there's a provision to do just this for the encoder inputs in Encoder.h:

void setup(uint8_t pin1, uint8_t pin2) {

ifdef INPUT_PULLUP

pinMode(pin1, INPUT_PULLUP); pinMode(pin2, INPUT_PULLUP);

else

pinMode(pin1, INPUT); digitalWrite(pin1, HIGH); pinMode(pin2, INPUT); digitalWrite(pin2, HIGH);

endif

INPUT_PULLUP doesn't seem to be defined anywhere, though. Where is the appropriate place for that to happen?

[blurfl]

INPUT_PULLUP is defined (I think by the Arduino Mega hardware file), so the encoder pins should be initialized as INPUT_PULLUP. Checking with a meter, this appears to be so.

[krkeegan]

I saw this happening tonight even on a machine with motors and encoders.

I am not sure that arduino defines the precompile INPUT_PULLUP I agree it defines the c variable INPUT_PULLUP.

I guess I can test this somewhat easily. I should have some time tomorrow to do so.

[krkeegan]

So good news/bad news.

Yes the INPUT_PULLUP is defined in ardiuno, so the encoder library is properly loading the pins using the pullup attribute.

Bad news: I watched this happen on my full system over the weekend. I don't have a good explanation for this, but I am worried that the machine may slowly slip out of calibration the more it is used.

I can add in some debugging to make it more apparent when this is happening and maybe take some video of it occurring.

[blurfl]

Is there a gcode sequence that is especially prone to this? What to watch for, how to trigger?

[krkeegan]

My first post on the issue seems to be the most reliable. The issue is a bit episodic, I don't know why. But restarting GC sometimes seems to help make it apparent.

[blurfl]

Using today's current master, when I 'Return to Center' then click on the 'Stop' repeatedly, I see Y change by +-0.01 after 8 or 9 clicks. I wonder if this has to do with the section of Motor.cpp write() where speed = 0? The merges committed this morning change the code in that area from an empty statement to setting the motor control pins to the LOW state, which should lock the motor if the Enable pin is high.

[krkeegan]

Maybe, I don't think so though, because the detach() function in motor already did this. And it is called by detachIfIdle() whenever there hasn't been any movement in two seconds.

You can also try setting one of your macros to send just a ?

This is ignored currently, but you can hammer away, sending 20-100 clicks without the firmware actually doing anything. I believe you will still see some of the same movement.

You can also try adding some debugging into settingsSaveStepstoEEprom(). I made three static variables and saved the current steps in them. Then if a change in steps was detected, it would print out the steps to the serial log. Sadly I didn't save this debugging as a branch. But it was helpful to see that the encoder positions are actually changing and often much more than is apparent from the sled position report.

[krkeegan]

Alright, mark me down as being crazy. I put in some good debugging code tonight, and I think we can close this as not existing.

I do see evidence that repeatedly pressing stop will cause slight changes in the reported position, generally only viewable in MM. The one axis moved .04mm total and the other .1mm total after pressing stop maybe 20-30 times. But after this the movements ceased no matter how many times I pressed anything.

During this whole process, the machine did not report any change in the step counts.

So what I am seeing is just the refinement of the position as reported by forward kinematics. Since forward kinematics now permits passing a guess location and we pass the current location when pressing stop, I am just seeing that refinement take place.

Anyways, sorry about screaming fire, turns out it was nothing (other than an issue with my test setup needing to be grounded).

[BarbourSmith]

So what I am seeing is just the refinement of the position as reported by forward kinematics. Since forward kinematics now permits passing a guess location and we pass the current location when pressing stop, I am just seeing that refinement take place.

I like that theory a lot, and I'm really glad that we have a theory for why it keeps moving. I'm glad that this one turned out to be a non-issue, but I'm also glad that we took the time to investigate. 👍 👍