If we have EMWA mode turned on, then the user wants to average
several samples from the temp sensors over time. But now we read
temp sensors only 4 times per second making this averaging take
much longer.
Read the temperatures continuously -- as fast as supported by the
probe type -- if we are using weight averaging (TEMP_EMWA < 1.0).
Heater PID loops must be called every 250ms, and temperature
probes do not need to be called any more often than that. Some
probes require some asynchronous operations to complete before
they're ready. Handle these in a state machine that first begins
the conversion and finally completes it on some future tick.
Signal it is complete by setting the new state variable to IDLE.
Kick off the heater PID loop by simply beginning the temperature
conversion on all the temperature probes. When each completes,
it will finish the process by calling its PID routine.
Remove the "next_read_time" concept altogether and just run each
temp conversion at fixed 250ms intervals.
Every type of temp sensor has its own special way to be read
and the once-simple loop has grown complex. Restore some sanity
by isolating the code for each sensor into its own inline
function.
In the process I noticed temp_flags is only ever set (never read)
and is used only in MAX6675. I don't understand what it was used
for, so for now, let's comment it out and revisit this later in
this series.
Integrate the next_read_time countdown into the loop as is common.
Check for start_adc() in the same loop -- before decrementing the
timer -- and call it when needed on the tick before we need the
results.
One concern I have still is that start_adc() may be called twice
within a few microseconds if two probes need to be read. I expect
it should only be called once, but I am not readily familiar with
the AVR ADC conversion protocol.
Extra clock next_start_adc_time was unnecessary. As @phord
observed, it was more understandable to explicitly call start_adc()
1 cycle ahead during temp_sensor_tick(), for sensors which use
analog_read().
As @phord observed, the conditions and the meaning of
next_read_time was not very intuitive. Changed that so that now
it represents the number of 10ms clock ticks before next sensor
reading, i.e. 1 is for 10ms, 2 for 20ms, etc.
Issues ADC reads (via start_adc()) only when actually needed.
When EWMA is enabled, then reads are performed at every
temp_sensor_tick() call, i.e. every 10ms. That's to ensure EWMA
gets updated frequently enough.
When EWMA is disabled, then the only other use for analog reads
is heater control, i.e. heater_tick(). It's run on a 250ms clock,
so that's how often analog reads need to be performed in that
case.
Additionally, ADC conversions are being started 10ms in advance,
so that they can finish before their results are read.
On AVRs analog conversions were running in essentially
free-running mode, i.e. next conversion was being started
immediately after the previous one ended. This caused many
unnecessary interrupts.
Now, conversions are initiated manually by start_adc(). This
causes a single read on each of the registered channels. Once
all channels are read, conversions stop until the next call of
start_adc().
Temperature residency time / temperature achieved check is in
the order of seconds, while updates for residency time were being
called every 10ms - unnecessarily often.
Thermal managers (PID, bang-bang, etc.) assume they're being
ticked on 250ms intervals. In reality, they were being updated on
each temperature reading, which was between 10ms and 250ms. This
caused thermal management to malfunction.
https://github.com/Traumflug/Teacup_Firmware/issues/211
The existing code eschews "break" apparently thinking that it will
exit the loop prematurely. In fact this will only exit the switch
correctly. The loop will run as normal.
Maybe it's not a big deal, though, since the sensors get re-inited
each time the type of sensor is encountered in the loop. Presumably
it's ok for each of these sensors to get inited multiple times. If
that's always the case, maybe there's no problem here. But with the
current code sensor types might be initialized even when there is no
sensor defined for that type if the TEMP_* boolean is #defined.
For example, if I have this in my config:
#define TEMP_MAX6675
#define TEMP_MCP3008
//#define TEMP_INTERCOM
DEFINE_TEMP_SENSOR(extruder, TT_MAX6675, AIO5, 0)
DEFINE_TEMP_SENSOR(bed, TT_MAX6675, AIO7, 0)
Then the MAX6675 initialization code will be run twice and the MCP3008
code will, too, even though no temp sensor is defined for that type.
This change does not address the "init runs twice" problem. It only
addresses the "initialization of unused types" problem.
Use a binary search to find our target temperate in fewer
comparisons. The search algorithm is ostensibly slower because
it involves a division, but it's a div-by-two so should be
optimized into a simple bit-shift. Fewer comparisons involves
fewer pgm_read_words and should be faster overall, but the gain
is arguably tiny.
Note by Traumflug:
According to @Wurstnase's performance measurements, gain is
actually pretty huge. Temp conversions 0..1024 before this
and the previous few commits:
minimum: 1011.84 clock cycles.
maximum: 1993.92 clock cycles.
average: 1768.85 clock cycles.
Now:
minimum: 437.72 clock cycles.
maximum: 494.76 clock cycles.
average: 470.278 clock cycles.
That's a speedup by factor 3.7 on average!
Save a division at runtime by pre-calculating the slope between each
pair of adjacent thermistortable values. Since we use the larger value
each time, save the slope between two values A and B in the table
with the B data. Therefore the slope is that between each value and
its predecessor in the list.
Store this new value in the third element of the now 3-integers-wide
array which makes up the table. Use fixed-point 6.10 format to store
the slope. This is almost too narrow for some slopes and maybe it
should be changed to 8.8 fixed-point. In practice this presents a
loss in accuracy, but it is still significantly better than the
previous fixed-sample-size table production method. In particular no
provision is made to handle values which scale over 65535, and it
seems we should at least warn about this if not simply fail before
letting the user go off compiling his code.
Add a new flag TEMPTABLE_FORMAT and define it as 1 to tell the code
that we are using this new and incompatible format. This lets us
tolerate old hand-crafted thermistor tables by keeping the slower
algorithm in case one is still used. New thermistor tables should be
defined with this new format and with the FORMAT define set accordingly.
With the default 25 samples this adds 100 bytes to the flash image for
the thermistortable storage for two different thermistors. But the
code is simplified and saves me 134 bytes in the bargain for a net
decrease in flash size of 34 bytes.
No code changes, but quite a few removals of __ARMEL_NOTYET__
guards. 20 such guards left.
Test: M105 should work and report plausible temperatures.
Current code size:
SIZES ARM... lpc1114
FLASH : 9460 bytes 29%
RAM : 1258 bytes 31%
EEPROM : 0 bytes 0%
The recent switch to send 'ok' postponed requires also sending a
newline in a few places, because this 'ok' is no longer at the
start of the line. Now it appears in its own line.
Some whitespace at line end was removed in heater.c.
Costs 14 bytes binary size on AVR.
Previously some features were excluded based on whether SIMULATOR
was defined. But in fact these should have been included when __AVR__
was defined. These used to be the same thing, but now with ARM coming
into the picture, they are not. Fix the situation so AVR includes are
truly only used when __AVR__ is defined.
The _crc16_update function appears to be specific to AVR; I've kept the
alternate implementation limited to AVR in that case in crc.c. I think
this is the right thing to do, but I am not sure. Maybe ARM has some
equivalent function in their libraries.
For now this is just a nice demonstration on how to send bytes
over SPI. Add SD_CARD_SELECT_PIN to your configuration board
file manually to see data signals on MOSI dancing on the scope.
The TODO's about SS in arduino*.h were wrong, SS does have a
chip-specific special meaning (used in SPI multi-master or SPI
slave mode). Still, a #define MAX6675_SELECT_PIN is missing.
Squashed in this commit from the SPI development topic branch to
get this first step working:
Author: jbernardis <jeff.bernardis@gmail.com>
2015-02-04 22:35:07
mendel.c: disable SPI in power management only when not needed.
If we want to talk to a SD card connected to SPI, we need SPI
powered, of course.
From Traumflug: nice catch, Jeff!
Order of heater and temperature sensor can differ, so they can get
different numbers. When printing extruder temperatur, we want to
print the temperature of the sensor matching the extruder heater,
not that of the temp sensor with the same number as the extruder
heater. Same for the bed respectively.
This should partially solve issue #96.
Problem spotted, described in a helpful manner and over-fixed
by @zungmann. This fix picks up his basic idea and implements it
with already existing state properties.
Test code which wants to customize config.h can do so without
touching config.h itself by wrapping config.h in a macro variable
which is passed in to the compiler. It defaults to "config.h" if
no override is provided.
This change would break makefile dependency checking since the selection
of a different header file on the command line is not noticed by make
as a build-trigger. To solve this, we add a layer to the BUILDDIR path
so build products are now specific to the USER_CONFIG choice if it is
not "config.h".
This code was accidentally removed long ago in a botched merge. This
patch recovers it and makes it build again. I've done minimal testing
and some necessary cleanup. It compiles and runs, but it probably still
has a few dust bunnies here and there.
I added registers and pin definitions to simulator.h and
simulator/simulator.c which I needed to match my Gen7-based config.
Other configs or non-AVR ports will need to define more or different
registers. Some registers are 16-bits, some are 8-bit, and some are just
constant values (enums). A more clever solution would read in the
chip-specific header and produce saner definitions which covered all
GPIOs. But this commit just takes the quick and easy path to support my
own hardware.
Most of this code originated in these commits:
commit cbf41dd4ad
Author: Stephan Walter <stephan@walter.name>
Date: Mon Oct 18 20:28:08 2010 +0200
document simulation
commit 3028b297f3
Author: Stephan Walter <stephan@walter.name>
Date: Mon Oct 18 20:15:59 2010 +0200
Add simulation code: use "make sim"
Additional tweaks:
Revert va_args processing for AVR, but keep 'int' generalization
for simulation. gcc wasn't lying. The sim really aborts without this.
Remove delay(us) from simulator (obsolete).
Improve the README.sim to demonstrate working pronterface connection
to sim. Also fix the build instructions.
Appease all stock configs.
Stub out intercom and shush usb_serial when building simulator.
Pretend to be all chip-types for config appeasement.
Replace sim_timer with AVR-simulator timer:
The original sim_timer and sim_clock provided direct replacements
for timer/clock.c in the main code. But when the main code changed,
simcode did not. The main clock.c was dropped and merged into timer.c.
Also, the timer.c now has movement calculation code in it in some
cases (ACCELERATION_TEMPORAL) and it would be wrong to teach the
simulator to do the same thing. Instead, teach the simulator to
emulate the AVR Timer1 functionality, reacting to values written to
OCR1A and OCR1B timer comparison registers.
Whenever OCR1A/B are changed, the sim_setTimer function needs to be
called. It is called automatically after a timer event, so changes
within the timer ISRs do not need to bother with this.
A C++ class could make this requirement go away by noticing the
assignment. On the other hand, a chip-agnostic timer.c would help
make the main code more portable. The latter cleanup is probably
better for us in the long run.
The requirement was simply neither obvious nor intuitive. Drawback
is, the indices of temperature sensors can now differ from these
of the heaters. That's easier to recognize for newbies, though.
Saves 10 bytes RAM and 16 bytes binary size on an 1284P (8 ADC
channels) running two sensors. Should also be a bit faster, as one
loop runs fewer iterations.
Also, checking for NUM_TEMP_SENSORS was a mistake, as temp sensors
_not_ using an analog pin exist. Extreme case is, temp sensors
exist, but zero ADC channels are used.
Remove the assumption there's always an extruder temperature
sensor and make reading on single sensors (e.g. M105 P2) more usable.
Apparently works very well, but *sigh* yet another 100 bytes of binary size.
This is useful when operation a milling spindle. In this case,
set the spindle speed with M104 Sxxx, where xxx is 1..255.
M104 S0 turns the spindle off.
Wurstnase
Phil Hord
Robert Konklewski
Markus Hitter
Witold Sowa
Phil Hord
jbernardis
David Forrest
Mikko Sivulainen