Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dyze High Temp Thermistor Support #4244

Merged

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Jul 9, 2016

Rebase, squash, cleanup, and improvement of #4098.

This is a follow up of : #2915 and #2941 (since changes that were made in MarlinDev are brought back in Marlin)

These modifications allow the use of high temperature thermistors in order to handle hot ends temperature up to 500 degrees Celsius.

Because thermistors able to read higher temperature tend to have a hard time reading lower values, two "defines" were added to allow a "preheating time" as well as handle some measure of noise reading (sudden drops for a few frames).

These defines have been added to the default configuration. Using another configuration or commenting these additions produce the same execution than before these changes.

A high temperature compatible thermistor has been added (no 66) to the thermistor table

The tests were taken from #2066
The hardware was : Board: Arduino MEGA2560 with RAMPS 1.4
Dyze Design sensor 500°C
HEATER_0_MINTEMP 21
MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED = 5;
MILLISECONDS_PREHEAT_TIME = 20000;

TEST 7 - Hotend's thermistor disconnected before startup
No error is shown. Temperature shown is 20°C, 1°C below the mintemp error.
Since the conditions «is preheating» and «target temperature > 0» is not met, the min_temp error is not shown as it would normally be.

TEST 9 - Hotend's thermistor disconnected after heating begins AND before (preheating time) seconds elapsed
Heater remains ON for the duration of "preheating time". Turns OFF once the duration is elapsed.

TEST 11 - Hotend's thermistor disconnected after heating begins AND after (preheating time) seconds elapsed
Min_temp error, heater is disabled.

TEST 13 - Hotend's thermistor disconnected after target temperature reached.
Min_temp error, heater is disabled.

@birkett
Copy link
Contributor

birkett commented Jul 9, 2016

An entry will need adding to thermistornames.h, for support in the Thermistor info menu.

@Edie47
Copy link

Edie47 commented Jul 9, 2016

Hi, I think I found some problem with activated SINGLENOZZLE feature. I turn on the printer. Then I set the temperature by M104 S190 T0 to 190°C. When the temperature gets steady I extrude 10mm. After that, I change the tool by T1. After these steps, the temperature starts rising and floats at 200-205°C. Switching the tool back by T0, the temperature again gets steady at the setpoint 190°C. Switching the tool by T1 temperature again rises to 200-205°C. Setting the temperature setpoint for T1 separately to 190°C does not help.

I found that this problem with temperature does not appear until the first extrusion and the tool switching to T1.

I haven't changed anything except some configuration for my Prusa i3. Here you find my branch https://github.com/Edie47/Marlin/tree/rc_dyze_thermistor_diff_test.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 10, 2016

@Edie47 That's very strange. I would not expect an issue like that to be specific to temperature sensor number 66. If such a bug exists, it is related to something else. Are you using AUTOTEMP at all? In other words, are you using M109 with an F temperature factor?

@Edie47
Copy link

Edie47 commented Jul 10, 2016

@thinkyhead I gave a try to another hotend that has 100k thermistor (temperature sensor number 1).The same thing happened so I can confirm that this issue is not connected with the temperature sensor number 66. I did not know it previously. I am using my extruder with temperature sensor number 66 as my default, therefore I posted it here.
No, I am not using AUTOTEMP at all.

EDIT: I also tried the RCBugFix branch from MarlinFirmware and the same problem occurs also there.

@thinkyhead
Copy link
Member Author

@Edie47 So, when you switch extruders, the target temperature on the display actually changes (immediately) too? I recently merged some additional changes into RCBugFix, so please give it another test with the most recent code and see if it's any different.

@thinkyhead thinkyhead merged commit 0ccc5d1 into MarlinFirmware:RCBugFix Jul 11, 2016
@thinkyhead thinkyhead deleted the rc_dyze_thermistor_diff branch July 11, 2016 01:08
@Edie47
Copy link

Edie47 commented Jul 11, 2016

@thinkyhead Yes, when I switch the extruders, the temperature on the display changes.
I tested the recent RCBugFix branch and the problem is still there. I made a video. Sorry for my English and some mistakes in the video. I am showing it in OctoPrint but I tested it also in RepetierHost and manually by entering gcode to set the temperature reference and extrude.

Hope it helps.

I have found a compilation error when #define MILLISECONDS_PREHEAT_TIME is uncommented in Configuration_adv.h. The problem was in temperature.h I corrected it. Take a look at my last commit of RCBugFix branch that I used for the test.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 11, 2016

Replace hotend with HOTEND_INDEX instead of e.
Try:

    static void setTargetHotend(const float& celsius, uint8_t e) {
      #if HOTENDS == 1
        UNUSED(e);
      #endif
      #ifdef MILLISECONDS_PREHEAT_TIME
        if (celsius == 0.0f)
-          reset_preheat_time(hotend);
-        else if (target_temperature[hotend] == 0.0f)
-          start_preheat_time(hotend);
+          reset_preheat_time(HOTEND_INDEX);
+        else if (target_temperature[HOTEND_INDEX] == 0.0f)
+          start_preheat_time(HOTEND_INDEX);
      #endif
      target_temperature[HOTEND_INDEX] = celsius;
      #if ENABLED(THERMAL_PROTECTION_HOTENDS) && WATCH_TEMP_PERIOD > 0
        start_watching_heater(HOTEND_INDEX);
      #endif
    }

@Edie47
Copy link

Edie47 commented Jul 11, 2016

I did. Unfortunately, it did not help.

@Blue-Marlin
Copy link
Contributor

Hmm, i thought so. There is more of this kind - somewhere. I'm still searching.

@Edie47
Copy link

Edie47 commented Jul 11, 2016

I tried now this. I turned on the printer. Then I sent the command M104 S190 T1. Waited for the temperature to get steady at 190°C. Then I extruded 10mm. After that, the temperature immediately increased and started floating between 198 and 205.
I disabled AUTOTEMP by commenting the define but it also did not help.

@Edie47
Copy link

Edie47 commented Jul 11, 2016

UPDATE: Commenting PID_ADD_EXTRUSION_RATE problem is gone and temperature is OK.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 12, 2016

temperature.cpp

-    long Temperature::last_position[HOTENDS];
+    long Temperature::last_position[EXTRUDERS];

!

@thinkyhead
woluld you please have a look at the use of lpq[]. lpq seems to me as if it is a ring buffer, where always only the last written value is used. If that is the case we can omit the complete buffer and can make it a single variable. Would make much more sense to me, to have a buffer, when the buffer would be summed up.

#if ENABLED(PID_ADD_EXTRUSION_RATE)
  cTerm[HOTEND_INDEX] = 0;
  if (_HOTEND_TEST) {
    long e_position = stepper.position(E_AXIS);
    if (e_position > last_position[_HOTEND_EXTRUDER]) {
      lpq[lpq_ptr++] = e_position - last_position[_HOTEND_EXTRUDER];
      last_position[_HOTEND_EXTRUDER] = e_position;
    }
    else {
      lpq[lpq_ptr++] = 0;
    }
    if (lpq_ptr >= lpq_len) lpq_ptr = 0;
    cTerm[HOTEND_INDEX] = (lpq[lpq_ptr] / planner.axis_steps_per_mm[E_AXIS]) * PID_PARAM(Kc, HOTEND_INDEX);
    pid_output += cTerm[HOTEND_INDEX];
  }
#endif //PID_ADD_EXTRUSION_RATE

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 12, 2016

If we want an average over the last few seconds her, a construct like:

uint16_t lpq = 0;

lpq += -lpq/lpq_len + (e_position - last_position[_HOTEND_EXTRUDER]);

would make more sense.

But temperature systems are slow. It makes much more sense to push all the energy needed as fast as possible. The slowness of the energy transfer will do the averaging anyway. Only if pid_output is already close to its max. (where it is clipped) would it make sense to distribute a bit.

uint16_t rest_cterm[HOTENDS] ={ 0 };
...
cTerm[HOTEND_INDEX] = (lpq[lpq_ptr] / planner.axis_steps_per_mm[E_AXIS]) * PID_PARAM(Kc, HOTEND_INDEX);
if ((cTerm[HOTEND_INDEX] +pid_output + rest_cterm[HOTEND_INDEX] ) >= PID_MAX ) {
  rest_cterm[HOTEND_INDEX] = cTerm[HOTEND_INDEX] +pid_output - PID_MAX;
  pid_output = PID_MAX;
}
else {
  pid_output = cTerm[HOTEND_INDEX] +pid_output + rest_cterm[HOTEND_INDEX];
  rest_cterm[HOTEND_INDEX] = 0;
}

or something like that.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 12, 2016

I suggest to disable PID_ADD_EXTRUSION_RATE by default. Without a proper DEFAULT_Kc, for a material, this does not make much sense.
#2885

@thinkyhead
Copy link
Member Author

Good catches.

I rechecked, and I think that's the last place where [e] needed to be replaced with [HOTEND_INDEX].

I'll see about disabling PID_ADD_EXTRUSION_RATE by default. I wonder if this actually helps out, even if it's a little off, just by adding a little more heat when the extruder is very active.

And yes, that is strange to have a ring buffer that only ever reads or writes the last value written. My assumption would be that it was intended to read from "some time in the recent past" and calculate based on that, to introduce a delay. But if it's not doing anything like that, well it's odd!

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 12, 2016

The reason that get_pid_output is using either active_extruder or e to index into last_position is because I was taking a cue from the MarlinKimbra version of SINGLENOZZLE. But thinking about it, what is the point? We only ever have a single E axis and thus only a single E position, right?

@thinkyhead
Copy link
Member Author

So, I peeked at the PID_ADD_EXTRUSION_RATE part of get_pid_output and actually the cTerm[HOTEND_INDEX] is not using the last-stored value, but actually the oldest value.

  lpq[lpq_ptr++] = 0;
}
// After incrementing above, lpq_ptr now points at the "next" value (the oldest one)
if (lpq_ptr >= lpq_len) lpq_ptr = 0;
cTerm[HOTEND_INDEX] = (lpq[lpq_ptr] / planner.axis_steps_per_mm[E_AXIS]) * PID_PARAM(Kc, HOTEND_INDEX);

@Blue-Marlin
Copy link
Contributor

Ah. Ok. Now the code seems to be correct. If it makes sense to use a delay is still questionable. Also if a experimental feature should be enabled by default.
#4278 looks good to me - for now.

@Edie47
Copy link

Edie47 commented Jul 12, 2016

I tested #4278 and the temperature regulation works fine. The problem is gone.

👍 Good job.

@Blue-Marlin
Copy link
Contributor

Thanks for the feedback.

@mjmeans
Copy link

mjmeans commented Jul 31, 2016

Is it possible to create a material Kc auto-tune function? So turn on and off the extruder while monitoring the temperature change rate. That way variances in material can be compensated for as at the time you are changing a spool of filament (heat up hot end, feed filament a few mm, then run filament auto-tune. Some wasted filament, but if it works, much better heat control.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 31, 2016

@mjmeans
When you tuned the lpq_len (M301 L) and could find good working Kc-values (M301 C) for at least 2 different materials, we can talk about automating the process.
I'm especially interested in you insights about, how short queue length improve long e-moves and large queues improve sequences of short-moves. Maybe you can find out whether a zero-length queue works best in all cases. Then we could drop the queue at all.
The key to improve this algorithm is to find a good test where we can see the waviness of the temperature, depending on long/short moves and the change of mass/second.

@Blue-Marlin
Copy link
Contributor

The idea "heat more when we have to melt more material" is simple. To find out, when exactly to heat more and how much exactly, to not make things worse, is a relative complex problem.

@thinkyhead
Copy link
Member Author

thinkyhead commented Aug 1, 2016

So… Do we agree it is best to disable by default? If so, renaming the option can help to ensure configurations are updated.

#if defined(PID_ADD_EXTRUSION_RATE)
  #error "PID_ADD_EXTRUSION_RATE is now PID_EXTRUSION_SCALING and is DISABLED by default. Are you sure you want to use this option? Please update your configuration."
#endif

@mjmeans
Copy link

mjmeans commented Aug 1, 2016

Perhaps it's as simple as running M303 again while extruding at a near max rate to arrive at new PID values, then interpolating based on amount about to be extruded.

@thinkyhead
Copy link
Member Author

@mjmeans The PID tuning function could get as detailed as we want. In the long run, it may be possible to introduce logic that adapts to fluctuations in temperature using a "dynamic hedging" algorithm. Either way, this constitutes a Feature Request and we should add it to our list of things to do for Marlin 1.2.

@Blue-Marlin
Copy link
Contributor

@thinkyhead
Switching off by default, is a reasonable plan. At least I can't see a difference in printing with the default parameters.

@ghost ghost mentioned this pull request Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants