repetier / Repetier-Firmware

Firmware for Arduino based RepRap 3D printer.
813 stars 734 forks source link

dev2 - Keeps resetting eeprom settings #915

Closed kyrreaa closed 4 years ago

kyrreaa commented 4 years ago

I've experienced multiple crashes now where dev2 decides to reset eeprom settings and end up replacing z height with defaults that are too large for machine. Result is massive head crash and damage to system. I've had to repair the hotend 4 times now. I guess the smart thing to do here is to set a default z-height that is shorter so if this happens again it will stop short of the bed. But, this worries me a lot.

I notice erratic behaviour such as hangs, busy-processing messages, resets and corrupt temperature readings that I suspect are memory corruption/overwrites. I think something is wrong here. My modifications to the main branch are quite small, and the reconfiguration to run on ramps-fd should not be major either.

Is anyone else experiencing such odditys?

repetier commented 4 years ago

So far I have no problems with my 3 printers running dev 2. RUMBA32 has no eeprom so it could in theory override settings with each upload (also that seems not to happen when flashing, so last 128 flash which I use as eeprom replacement do not get deleted on flash).

Shorter height would be a good idea as default, or the correct height if it does not change.

kyrreaa commented 4 years ago

A printjob that is aborted or ended after using babysteps to adjust on first layer also seems to end up in a infinite processing loop. And there's apparantly something with my axis modification that adds motion to extruder if extruder is hot on a homing.

repetier commented 4 years ago

And there's apparantly something with my axis modification that adds motion to extruder if extruder is hot on a homing.

I thought your other problem is extrusion during homing. What is that axis modification that you did? It is important to keep all 3 coordinate systems in sync or you will get unexpected motions sooner or later. That is something I assume also happens when homing extrudes and you get different speeds. The extra move causes then a slowdown.

kyrreaa commented 4 years ago

Yes I think the motion is related to unsync. In PrinterTypeCoreXYZ.cpp I found that this function for transform did not work and instead of fixing it I replaced it.

void PrinterType::transform(float pos[NUM_AXES], int32_t motor[NUM_AXES]) {
    motor[X_AXIS] = lroundl((COREXYZ_X_X * pos[X_AXIS] + COREXYZ_X_Y * pos[Y_AXIS] + COREXYZ_X_Z * pos[Z_AXIS]) * Motion1::resolution[X_AXIS]);
    motor[Y_AXIS] = lroundl((COREXYZ_Y_X * pos[X_AXIS] + COREXYZ_Y_Y * pos[Y_AXIS] + COREXYZ_Y_Z * pos[Z_AXIS]) * Motion1::resolution[Y_AXIS]);
    motor[Z_AXIS] = lroundl((COREXYZ_Z_X * pos[X_AXIS] + COREXYZ_Z_Y * pos[Y_AXIS] + COREXYZ_Z_Z * pos[Z_AXIS]) * Motion1::resolution[Z_AXIS]);
#if defined(COREXYZ_Z_X) && defined(COREXYZ_Z_X) && defined(COREXYZ_Z_X) && NUM_AXES > A_AXIS
    motor[Z_AXIS] = lroundl((COREXYZ_A_X * pos[X_AXIS] + COREXYZ_A_Y * pos[Y_AXIS] + COREXYZ_A_Z * pos[Z_AXIS]) * Motion1::resolution[Z_AXIS]);
    motor[E_AXIS] = lroundl(pos[E_AXIS] * Motion1::resolution[E_AXIS]);
    for (fast8_t i = B_AXIS; i < NUM_AXES; i++) {
        motor[i] = lroundl(pos[i] * Motion1::resolution[i]);
    }
#else
    for (fast8_t i = E_AXIS; i < NUM_AXES; i++) {
        motor[i] = lroundl(pos[i] * Motion1::resolution[i]);
    }
#endif
}

to:

void PrinterType::transform(float pos[NUM_AXES], int32_t motor[NUM_AXES]) {
    motor[X_AXIS] = lroundl((COREXYZ_X_X * pos[X_AXIS] + COREXYZ_X_Y * pos[Y_AXIS] + COREXYZ_X_Z * pos[Z_AXIS]) * Motion1::resolution[X_AXIS]);
    motor[Y_AXIS] = lroundl((COREXYZ_Y_X * pos[X_AXIS] + COREXYZ_Y_Y * pos[Y_AXIS] + COREXYZ_Y_Z * pos[Z_AXIS]) * Motion1::resolution[Y_AXIS]);
    motor[Z_AXIS] = lroundl((COREXYZ_Z_X * pos[X_AXIS] + COREXYZ_Z_Y * pos[Y_AXIS] + COREXYZ_Z_Z * pos[Z_AXIS]) * Motion1::resolution[Z_AXIS]);
    motor[A_AXIS] = lroundl((COREXYZ_A_X * pos[X_AXIS] + COREXYZ_A_Y * pos[Y_AXIS] + COREXYZ_A_Z * pos[Z_AXIS]) * Motion1::resolution[A_AXIS]);
    for (fast8_t i = E_AXIS; i < NUM_AXES; i++) {
        if (i != A_AXIS) {
            motor[i] = lroundl(pos[i] * Motion1::resolution[i]);
        }
    }
}

I see now that the defined() statements in the upstream is copy-paste errored. Maybe this is related? Should they refer to COREXYZ_A_X ?

kyrreaa commented 4 years ago

I also found that babysteps only moved z motor. Changing PrinterType::M290 to this worked but whenever I use babystems the printer hangs on processing after I cancel a job.

void PrinterType::M290(GCode* com) {
    InterruptProtectedBlock lock;
    if (com->hasZ()) {
        float z = constrain(com->Z, -2, 2);
        Motion2::openBabysteps[X_AXIS] += z * COREXYZ_X_Z * Motion1::resolution[X_AXIS];
        Motion2::openBabysteps[Y_AXIS] += z * COREXYZ_Y_Z * Motion1::resolution[Y_AXIS];
        Motion2::openBabysteps[Z_AXIS] += z * COREXYZ_Z_Z * Motion1::resolution[Z_AXIS];
        Motion2::openBabysteps[A_AXIS] += z * COREXYZ_A_Z * Motion1::resolution[A_AXIS];    }
}
repetier commented 4 years ago

You are right that A axis was not added correctly. It should be

void PrinterType::transform(float pos[NUM_AXES], int32_t motor[NUM_AXES]) {
    motor[X_AXIS] = lroundl((COREXYZ_X_X * pos[X_AXIS] + COREXYZ_X_Y * pos[Y_AXIS] + COREXYZ_X_Z * pos[Z_AXIS]) * Motion1::resolution[X_AXIS]);
    motor[Y_AXIS] = lroundl((COREXYZ_Y_X * pos[X_AXIS] + COREXYZ_Y_Y * pos[Y_AXIS] + COREXYZ_Y_Z * pos[Z_AXIS]) * Motion1::resolution[Y_AXIS]);
    motor[Z_AXIS] = lroundl((COREXYZ_Z_X * pos[X_AXIS] + COREXYZ_Z_Y * pos[Y_AXIS] + COREXYZ_Z_Z * pos[Z_AXIS]) * Motion1::resolution[Z_AXIS]);
#if defined(COREXYZ_A_X) && defined(COREXYZ_A_Y) && defined(COREXYZ_A_Z) && NUM_AXES > A_AXIS
    motor[Z_AXIS] = lroundl((COREXYZ_A_X * pos[X_AXIS] + COREXYZ_A_Y * pos[Y_AXIS] + COREXYZ_A_Z * pos[Z_AXIS]) * Motion1::resolution[Z_AXIS]);
    motor[E_AXIS] = lroundl(pos[E_AXIS] * Motion1::resolution[E_AXIS]);
    for (fast8_t i = B_AXIS; i < NUM_AXES; i++) {
        motor[i] = lroundl(pos[i] * Motion1::resolution[i]);
    }
#else
    for (fast8_t i = E_AXIS; i < NUM_AXES; i++) {
        motor[i] = lroundl(pos[i] * Motion1::resolution[i]);
    }
#endif
}

And for babysteps correctly:

void PrinterType::M290(GCode* com) {
    InterruptProtectedBlock lock;
    if (com->hasZ()) {
        float z = constrain(com->Z, -2, 2);
        Motion2::openBabysteps[X_AXIS] += z * COREXYZ_X_Z * Motion1::resolution[X_AXIS];
        Motion2::openBabysteps[Y_AXIS] += z * COREXYZ_Y_Z * Motion1::resolution[Y_AXIS];
        Motion2::openBabysteps[Z_AXIS] += z * COREXYZ_Z_Z * Motion1::resolution[Z_AXIS];
#if defined(COREXYZ_A_X) && defined(COREXYZ_A_Y) && defined(COREXYZ_A_Z) && NUM_AXES > A_AXIS
        Motion2::openBabysteps[A_AXIS] += z * COREXYZ_A_Z * Motion1::resolution[A_AXIS];
#endif
    }
}

To suffice also correctly printers without A axis. Having A also added is a very special case of you. Did you in configuration extend

define BABYSTEPS_PER_BLOCK \

{ 1, 1, 1 }

to

define BABYSTEPS_PER_BLOCK \

{ 1, 1, 1, 1, 1 }

to fit 5 dimensions with correct values. Otherwise A axis has a random value which might cause the problem in your case.

Can you freshen up my mind what A axis was used for here? I know you told me and I added that stuff afterwards, but do not remember what is was for any more.

kyrreaa commented 4 years ago

I forgot to extend the BABYSTEPS_PER_BLOCK! And I discovered that if there is a ack missing on eeprom during init it will fail to init and hang. I had a loose wire to my eeprom (which is an extension of ramps-fd not onboard) and it was causing the resets I think. The code could potentially be more robust against ack errors etc but oh well. (For example, in my normal code for work stuff I always check for ack success otherwise return ack fail. This allows me to not only continue in case of missing units on bus but also scan bus for units.)

About my added code. My machine is a real Core-XYZ using all 4 axis in the transform. Ideally one would call the motors Motor1, Motor2, Motor3 and Motor4 (or start at 0) as A, B etc would confuse them being gcode axis of rotation etc. My additions do what your code now does with the define fixed. It is otherwise generic and correct I think and I will clean up my code and revert to the upstream to be in sync.

repetier commented 4 years ago

Yes, missing ACK will trigger watchdog and reset, which is then what is probably happening. For communication we use the Wire library since all platforms have it making it easier and that is waiting for ACK. On the other side missed ACK will also break something in many cases. So question is what is better - always fail hard or not register something was changed.

And last thing - when it happens while writing to eeprom checksum is wrong and eeprom will reset, just what you experienced.

Please let me know if the homing problem still exists, then I will have a look when I get some more time.

kyrreaa commented 4 years ago

Great! I will be running new tests now after I sync with upstream again. Will make sure this code has as few changes as possible and ideally only to the config and userpins.

kyrreaa commented 4 years ago

Tested in synch with dev2 after correcting BABYSTEPS_PER_BLOCK in config. Extruder motion still happens when heated.

kyrreaa commented 4 years ago

I will close this as the eeprom thing is solved as loose wire.