Hotend PID fails to reach target when Auto Bed Level feature is turned on.

[Micro-MTW]

Arduino 1.6.6
Rambo Controller.
40 watt heater Cartridge
Type 5 thermistor

Using RC6 and RCbugfix I have found the target temp of 230C is not reachable when the auto bed level feature is enabled. As soon as temp exceeds 220C and enters PID it never will go past 222C. If I disable PID control it has no issue reaching target. If I disable ABL it has no issue reaching target under PID.

I ran a PID tune and and it worked properly and inserted the values and it will still not reach target as long as ABL is turned on.

I was using a 60 watt heater and a type 1 thermistor with RC 6 and it worked properly.

This has been a very strange issue and I havent seen any other mention of it here.
I have replaced the controller, the thermistor, and the heater cartridge and the issue remains.

RC3 works correctly.

Thanks for all the hard work!

[Roxy-3D]

I'm seeing a similar issue with RCBugFix except I'm only trying to go to 205 C. I can't get there. And it looks like it is struggling to keep creeping the heat up only to have the heat turned off and it drops 10 degrees. And then the temperature starts creeping back to where I want it. In fact, I can't even get it to complete one cycle of the M303 PID Tuning. I never get to temperature and complete a cycle.

I have a very early RC6 that does not show the problem. So my question is "Did we do anything to the PID algorithms and/or the heater code?" If so, we better double check those changes.

[jbrazio]

What values are you using for:

#define BANG_MAX
#define PID_MAX
#define PID_FUNCTIONAL_RANGE
#define PID_INTEGRAL_DRIVE_MAX 

PS: I assume PID is tuned.

[Micro-MTW]

I have tried 150 to 255 on bang max.

#define PID_MAX BANG_MAX

I have used the stock value of 10 on PID_FUNCTIONAL_RANGE but have also tried 15 and 20. The system behaves the same. As soon as it hands off control to the PID from BANG it just hovers right above that threshold.

Using the stock value in RC for the #define PID_INTEGRAL_DRIVE_MAX

And yes I have PID tuned. Tthe PID tuning cycle works properly and generates a set of values that I have inserted but the issue remains.

It is acting like the duty cycle of the output is no where near where it needs to be during PID. If needed I can toss a scope on that pin and verify what its doing. But with just a meter attached I can tell soon as the PID takes over the duty time drops quite a bit until it goes below the functional range then jumps back up for a bit as Bang takes over. That cycle repeat over and over..

Thanks for taking a look.

[Roxy-3D]

I have these values:

#define PIDTEMP
#define BANG_MAX 255 // limits current to nozzle while in bang-bang mode; 255=full current
#define PID_MAX BANG_MAX // limits current to nozzle while PID is active (see PID_FUNCTIONAL_RANGE below); 255=full current
#if ENABLED(PIDTEMP)
  //#define PID_AUTOTUNE_MENU // Add PID Autotune to the LCD "Temperature" menu to run M303 and apply the result.
  //#define PID_DEBUG // Sends debug data to the serial port.
  //#define PID_OPENLOOP 1 // Puts PID in open loop. M104/M140 sets the output power from 0 to PID_MAX
  //#define SLOW_PWM_HEATERS // PWM with very low frequency (roughly 0.125Hz=8s) and minimum state time of approximately 1s useful for heaters driven by a relay
  //#define PID_PARAMS_PER_HOTEND // Uses separate PID parameters for each extruder (useful for mismatched extruders)
                                  // Set/get with gcode: M301 E[extruder number, 0-2]
  #define PID_FUNCTIONAL_RANGE 10 // If the temperature difference between the target temperature and the actual temperature
                                  // is more than PID_FUNCTIONAL_RANGE then the PID will be shut off and the heater will be set to min/max.
  #define PID_INTEGRAL_DRIVE_MAX PID_MAX  //limit for the integral term
  #define K1 0.95 //smoothing factor within the PID

[jbrazio]

Before digging into code, could you please increase the order of magnitude of the functional range from 10 to 100 ? To check if the behavior changes i.e. the delta to set point ? You could also test delivering full control to the PID by setting the functional range to 250 or so ?

[Roxy-3D]

OK... But I have to wait until the printer is done with its print!

[jbrazio]

So @Roxy-3D you don't have the issue always ?

[Roxy-3D]

I don't have the issue with a much older RC-6. And part of the problem is I'm not even sure it is a problem with the current RC-6. I've cut into and changed a lot of things for the Unified Bed Leveling. I was to the point where I wanted to test the planner adapting to what was stored in the Mesh. And I couldn't get up to temperature.

It is possible I screwed something up. But the one place I have not been is any where near the Thermal stuff. (At least, not intentionally!) I'm moving my code over to the latest RC-6 and it will be interesting to see how the hot end behaves once I get that done.

[thinkyhead]

@Micro-MTW Strange that ABL should have any bearing on this, unless it's a compiler quirk. Can you try building with Arduino 1.6.0 to see if it works any better? (We've found 1.6.0 sometimes fixes issues that appear with 1.6.8 and 1.6.9).

[Roxy-3D]

I'll know more when I finished getting moved over to the latest RCBugFix.
And most likely it has nothing to do with ABL. It is more likely I
acidently deleted something I need. Or maybe I've caused another problem
by taking too much time in the Interrupt Routines.

On Sun, Jun 19, 2016 at 7:23 PM, Scott Lahteine [email protected]
wrote:

Strange that ABL should have any bearing on this, unless it's a compiler
quirk. Can you try building with Arduino 1.6.0 to see if it works any
better? (We've found 1.6.0 sometimes fixes issues that appear with 1.6.8
and 1.6.9).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4086 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AIA9YkH_6Irh26BkEgIh7MQjIvclzmsjks5qNd2VgaJpZM4I5Fr2
.

[thinkyhead]

I wonder why the software PID handling isn't "impervious" to slow interrupts or other timing issues. Perhaps there is a way to make it more robust.

[Micro-MTW]

As per request on trying different values for functional range.
The below were after running a PID tune.

at 10 PID fails
at 20 PID fails
at 30 50/50 shot of it working
at 50 PID works
at 100 PID works.

[Roxy-3D]

@Micro-MTW I have some more information on what I'm seeing. When the temperature is climbing towards the set value, when it gets exactly 10 C degrees below the desired Temp I see one of two behavior. Some times the desired temp gets changed to 0. Other times, Marlin reboots.

Do you see that behavior?

[Micro-MTW]

When it hits the functional range (when set to 10 , 15, or 20) I see the temp bump a couple degrees more then it cools off, then bang kicks in and bumps up a few degrees and starts cooling off and back again to bang handling. It does this until it errors out with the failure to reach temp. I actually have not looked that close at what the requested temp is in the logs.

I am using bugfix from two days ago and as long as I keep the functional range to 50 or greater it works. The ABL seems to work better under bugfix than RC3 so i am still using it :)

[Grogyan]

Please consider trying another heater cartridge, I've had to replace mine which wasn't heating up enough a while ago, and I had thought it was a problem with firmware, which it wasn't, just the heater cartridge just failed to get hot enough.

[Roxy-3D]

@Micro-MTW What you describe is very similar to what I saw. Once the temperature got within the functional range, the heaters turned off. But the temperature kept rising for a little bit just because there is a time delay between the sensors and the heater.

You can turn on PID_DEBUG and see what the PID terms are and what the output to the heater is. My guess is it is dropping to 0 until it is outside the functional range.

I think you might want to try doing a M502 followed by a M500. It is possible your PID constants are corrupted in the EEPROM.

[Micro-MTW]

Grogyan,

As stated in my first post. I have replaced the cartridge, thermistor, and Rambo controller before I even started looking for a firmware issue.

The problem follows RC6 and RCBugfix but works properly with RC3.

[Micro-MTW]

Roxy-3D

Will give the EEprom wipe a shot and let ya know.

[Micro-MTW]

Roxy-3D

The wipe may have fixed it. moved it to 10 on the functional range after wiping the EE and it came up to temp normally two times in a row. I will keep messing with it. I looked at the stored PID values before and after and i could not see any difference on the listed line on system boot, But something must have changed. Still freaky that it somehow got twisted into the ABL enabled or not.

Thanks, will keep you informed.

[Roxy-3D]

I don't know how it happened in your case, but the EEPROM must have gotten corrupted. In my case, I know how it happened. I moved the storage block from 0x0100 to 0x010. I did that to consolodate space and give me more room to store bed mesh's.

[thinkyhead]

Definitely be sure to do M300 U1 every so often, and re-check the current values, as changes in the firmware's interrupt timing or other nuances could have some bearing on the PID values, requiring a re-tune even though the hardware is the same.

[Micro-MTW]

Roxy-3D,

Little update on the temp target issue. One of the fellas that works for me installed RC6 on one of his machines and had the identical problem that i outlined above. He tried the EEprom wipe with RC6 and it didnt not correct the issue. I had him install the bugfix version i downloaded on 6/19/16. He still had the temp issue. I then had him do the EEprom wipe and the temp started acting properly.

Dont know if this helps you any but thought it was worth sharing with you.

[Roxy-3D]

Don't know if this helps you any but thought it was worth sharing with you.

This EEPROM issue never seems to go away. There is even code that checks to make sure the running firmware version matches the version that created the EEPROM data. If not, it should not load the 'corrupted' data. And almost all of the stored information is printed out when Marlin boots. That is another 'safety check'. But neither of us noticed the wrong values being displayed here:

        CONFIG_ECHO_START;
        SERIAL_ECHOPAIR("  M301 P", PID_PARAM(Kp, 0)); // for compatibility with hosts, only echo values for E0
        SERIAL_ECHOPAIR(" I", unscalePID_i(PID_PARAM(Ki, 0)));
        SERIAL_ECHOPAIR(" D", unscalePID_d(PID_PARAM(Kd, 0)))

And I most certainly know about this issue and I deliberately shifted the position of where the data was going to be stored in my EEPROM. Some how... I fell into the trap of thinking the Thermal code wasn't behaving correctly.

I then had him do the EEprom wipe and the temp started acting properly.

I'm not sure I like the word 'wipe' here. (Wipe kind of implies we are clearing it.) What we really are doing is re-loading the EEPROM with the configured default values. I presume it was a M502 and M500 that fixed your friend's machine? Any way... All's well that ends well!

[Micro-MTW]

Roxy-3D. Yeah wipe is not the proper term. But yeah a M502 followed with an M500 did the trick with bugfix. With RC6 it didnt help.

[thinkyhead]

It sounds possibly like some variable is being used uninitialized, or possibly there is a buffer overrun someplace stepping on configured values. Surprising if it persists still, since RC6.

@Micro-MTW In the process of examining this error, did you happen to check the current PID values on the LCD controller or with M503 to see if they were corrupted?

[Roxy-3D]

It sounds possibly like some variable is being used uninitialized, or possibly there is a buffer overrun someplace stepping on configured values.

Another possibility is the offsets of the stuff at the end of the storage block got shifted to another location. I did a quick check and didn't see anything. But if the version number stayed the same but the offsets moved, the values would be corrupted and still be pulled out of the EEPROM. The Ki, Kd and Kp parameters are at the tail end of the storage block so this makes the theory a tiny bit more plausible. I guess we could load up the original RC6 and compare how much storage it says is in use compared to the latest RCBugFix. If the size isn't identical, we need to bump the version number.

[Micro-MTW]

thinkyhead,

Actually yes I did verify the values of the Pid were what i had set them to in Marlin after a pid tune. No lcd on the printer but I did use the text dump that hits the terminal on startup to verify the values.

Almost like the pointer to the pid values is getting hosed.

[thinkyhead]

@Micro-MTW Was that at the point where you were still seeing the aberrant behavior?

Thing is, when using M503 it shows the values in RAM and not the values stored in EEPROM. So, to really confirm that the EEPROM is corrupted you have to use M501 followed by M503 — before using M502, M300, or M301, and before using M500 to write, since that could correct the EEPROM.

[thinkyhead]

@Roxy-3D When we add code to write to EEPROM, we have to make sure that no option can affect the EEPROM layout. So, for example, if you change the number of grid points in Mesh Bed Leveling, the EEPROM should still always store the "maximum" number of points (10x10, for example). I believe some of the EEPROM code is incorrectly implemented, so that some options are affecting how much data is being written and read. So if you change the mesh grid size in your config and flash, EEPROM will not read correctly.

We don't currently confirm that the EEPROM data size matches some expected size, but then it would be a hassle to keep a "known size" updated in the sources. So a better option is to scan the data, as it is written, generate a checksum, and store the checksum in EEPROM as the last step. Then we can verify the checksum. Then if the data is too long or short, the checksum won't match.

[Roxy-3D]

Right! I understand. What I took a quick look at was to make sure all the changes in data type names did not change the size of the item being stored. One that I wasn't sure about was the bool type. (I think it was mbl.active that shifted from a bool to uint8) My guess was it would be represented internally as an int by the compiler just because the compiler has an infatuation with int's. So i double checked that because it would mess everything up.

But that turned out to be a rabbit trail that didn't lead anywhere. bool's turn into an 8-bit char and that did not cause any offsets to shift.

[thinkyhead]

I've added a checksum to EEPROM with #4167.
Give it a test and see how it works. (I haven't been able to test it yet.)

[lrpirlet]

Why not write a eeprom_structure.h file that would contain the
TIMESTAMP (this macro is a string constant that gives the date and
time of the last modification of the current source file)... Any time a
change is done to the EEPROM structure, the TIMESTAMP would not match
and it would be easy to issue a warning to run m500 m502...
(In fact my only question is that I have no idea if git keep the
modification date when swapping files as a result of a git command).

2016-06-29 3:25 GMT+02:00 Scott Lahteine [email protected]:

I've added a checksum to EEPROM with #4167
#4167.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#4086 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALEmPZR2UO62PHuTx1zL9ifpaIqe_7KSks5qQcmjgaJpZM4I5Fr2
.

[Roxy-3D]

One problem with doing that is any bug fixed in the firmware does not necessarily change the EEPROM storage format. People would lose their settings anytime they up graded the firmware. And as a result of that, people would start refusing to update the firmware if it worked at all for them.

[lrpirlet]

@Roxy-3D You are right, and worse, a git checkout does change the date on the file...

Now, when I change my config file to increase the number of probe points in G29, I run the risk to overwrite some values because the number of bytes occupied by Mesh Bed Leveling values is function of the number of probe points...
Today, I overwrite the m851 value when I increase the number of probe points and recompile...
I read a corrupted value for m851 when I decrease the number of probe points and recompile...

IMHO, it would be better to group to the top all "fixed lenght" values, common to all printers.
Then values that fits in a fixed lenght structure
Then, at the end I would place the Mesh bed leveling.
So we would place the M851 value right after the common values

Each group would be a structure including a magic number, a checksum and a count of the used bytes... So we could "salvage" some part of the eeprom would some change "corrupt" the EEPROM

This is today layout..

/**
 * V23 EEPROM Layout:
 *
 *  100  Version (char x4)
 *
 *  104  M92 XYZE  planner.axis_steps_per_mm (float x4)
 *  120  M203 XYZE planner.max_feedrate (float x4)
 *  136  M201 XYZE planner.max_acceleration_mm_per_s2 (uint32_t x4)
 *  152  M204 P    planner.acceleration (float)
 *  156  M204 R    planner.retract_acceleration (float)
 *  160  M204 T    planner.travel_acceleration (float)
 *  164  M205 S    planner.min_feedrate (float)
 *  168  M205 T    planner.min_travel_feedrate (float)
 *  172  M205 B    planner.min_segment_time (ulong)
 *  176  M205 X    planner.max_xy_jerk (float)
 *  180  M205 Z    planner.max_z_jerk (float)
 *  184  M205 E    planner.max_e_jerk (float)
 *  188  M206 XYZ  home_offset (float x3)
 *
 * Mesh bed leveling:
 *  200  M420 S    status (uint8)
 *  201            z_offset (float)
 *  205            mesh_num_x (uint8 as set in firmware)
 *  206            mesh_num_y (uint8 as set in firmware)
 *  207 G29 S3 XYZ z_values[][] (float x9, by default)
 *
 * AUTO BED LEVELING
 *  243  M851      zprobe_zoffset (float)
 *
 * DELTA:
... more saved values

I would suggest

/**
 * V23 EEPROM Layout:
 *
 *  100  Version (char x4)
 *
 *  magic number =  Common saved value
 *  count of bytes for checksum = 96
 *  Checksum from 105 to 200
 *  104  M92 XYZE  planner.axis_steps_per_mm (float x4)
 *  120  M203 XYZE planner.max_feedrate (float x4)
 *  136  M201 XYZE planner.max_acceleration_mm_per_s2 (uint32_t x4)
 *  152  M204 P    planner.acceleration (float)
 *  156  M204 R    planner.retract_acceleration (float)
 *  160  M204 T    planner.travel_acceleration (float)
 *  164  M205 S    planner.min_feedrate (float)
 *  168  M205 T    planner.min_travel_feedrate (float)
 *  172  M205 B    planner.min_segment_time (ulong)
 *  176  M205 X    planner.max_xy_jerk (float)
 *  180  M205 Z    planner.max_z_jerk (float)
 *  184  M205 E    planner.max_e_jerk (float)
 *  188  M206 XYZ  home_offset (float x3)
 *
... more fixed lengh structures...
 *
 *  magic number = ULTIPANEL
 *  count of bytes for checksum = 24
 *  Checksum for "ULTIPANEL range of value"
 *  m+7  M145 S0 H plaPreheatHotendTemp (int)
 *  m+9  M145 S0 B plaPreheatHPBTemp (int)
 *  m+1  M145 S0 F plaPreheatFanSpeed (int)
 *  m+3  M145 S1 H absPreheatHotendTemp (int)
 *  m+5  M145 S1 B absPreheatHPBTemp (int)
 *  m+7  M145 S1 F absPreheatFanSpeed (int)
 *
...  more fixed lengh structures...
 *
 * magic number = Mesh bed leveling
 *  count of bytes for checksum = n+7+sizeof float*number of probe points
 *  Checksum for "Mesh bed leveling range of value"
 *  n+0  M420 S    status (uint8)
 *  n+1            z_offset (float)
 *  n+5            mesh_num_x (uint8 as set in firmware)
 *  n+6            mesh_num_y (uint8 as set in firmware)
 *  n+7 G29 S3 XYZ z_values[][] (float x9, by default)
* 

Obviously, some synchronization with host software such as repetier that is able to read and write EEPROM may be needed...

[Roxy-3D]

Now, when I change my config file to increase the number of probe points in G29, I run the risk to overwrite some values because the number of bytes occupied by Mesh Bed Leveling values is function of the number of probe points...

In the new Bed Leveling system, the Bed Leveling data is stored at the end of the EEPROM. That is done for the explicit purpose of 'protecting' it from any other changes in the storage of configuration data. The Bed Leveling Mesh's take an investment of time to get right. So you don't want to lose those just because something else is deemed worthy of being saved in the EEPROM.

Right now changing the size of a Mesh will cause the previously stored Mesh's to be deleted. I don't think I'm going to relax that constraint right away. Probably, the right answer is to tell the user to err on the side of higher resolution mesh.

[jbrazio]

@thinkyhead, @Roxy-3D can we close this issue ?

[thinkyhead]

changing the size of a Mesh will cause the previously stored Mesh's to be deleted

Perhaps in future, all the meshes and grids will be adjustable at runtime.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.