Tronxy/Cstar P802E

[rkonklex]

Hi! I'm a software developer and a recent owner of a Chineese Prusa i3 variant, namely the Tronxy/Cstar P802E. For starters, I'm looking to work on Teacup to make it run on the printer, and to make the printer perform at its best. I've already got it running and made a few minor improvements to Teacup. I'd like write access to the repo (a branch al least) so that I can share it for a possible merge into the main branch in the future.

Like I said above, there are already a couple of improvements:

• board & printer configuration for the P802E
• fixed linker error in watchdog.c
• an option to oversample ADC readings, based on AVR121: Enhancing ADC resolution by oversampling

Next, I'm looking to enable selection of the main timer. Currently it's hard-coded to the 16-bit TIMER1. The problem is, on my board its PWM outputs are used to control the bed and the extruder heaters, so currently I'm left with on/off control only. The chip on this board is ATmega1284p, which has another 16-bit timer - TIMER3, which is unused, in my case. Moving the clocks to TIMER3 would free up TIMER1, which could be then used for heater PWM.
I'd make this feature optional, of course.

[Traumflug]

Hello @nythil, welcome to Teacup development, I just added you as collaborator. We had some discussion by email already. The rules are simple: work on existing branches only in agreement with the creator of the banch. For yur own work, create as many branches as you feel need for. In Git parlance these branches are known as "topic branches".

Regarding your plans:

1. Yes, good idea to add a board and printer configuration for the P802E.
2. Link errors should be fixed, of course, your commit (on a branch) is welcome.
3. Teacup features two algorithms for enhancing ADC readings already: a temperature history, see code around (or wrapped by) TH_COUNT, and an exponential moving average, see code around TEMP_EWMA. A third one on top of this is likely not the ideal solution. Instead, a new algorithm, in case it's actually better, should replace the existing two.

Now, a moving average is almost the same as oversampling. Difference is that oversampling moves the decimal a few bits to the left for more accuracy, which a moving average can do as well. This comment, Lesson # 3, shows how to implement such a moving average without expensive divisions. Divisions by 2, 4, 8, 16, ... are cheap, because a bit-shift to the right.

More work about temperature readings is currently going on here: #208

Regarding the timer thing I better keep shut, else I'd yell about how stupid wheel-reinventions are.

[rkonklex]

Hello. Thanks for having me. Rules for collaboration are clear. I just pushed a batch of changes on my branch.

I didn't quite get the last part. Why is selecting a different timer reinventing the wheel? I know that TIMER1 is both 16-bit and high interrupt priority, and it's the optimal choice performance-wise. But I'm stuck with a SMT all-integrated board, no possibility of modifications. Some guy far away decided to use TIMER1 for PWM control, so I'm left with TIMER3 for stepping. Allowing for the possibility of timer selection isn't that hard and all the work is done by the preprocessor and the compiler. Is that so bad?

[Traumflug]

I just pushed a batch of changes on my branch.

Oh, that's quick. Thank you very much!

Cherry picked three and a half of them. Now you can rebase your branch with --ignore-whitespace to experimental again to remove the duplicates.

Regarding board.tronxy.h I have a question: Does this board actually allow to use different CPU clocks? Just asking before committing, because that's a rare feature.

Regarding this oversampling thing: well, see above, I'm not exactly keen on adding a third algorithm on top of the two existing ones. Also, unless I'm mistaken, it doesn't raise resolution by doing this oversampling, it just averages a number of readings.

The usage of maxAdc commit is left for reviewing tomorrow. Ideally we'd pick this value from header files somewhere, because that's a fixed MCU property.

Why is selecting a different timer reinventing the wheel?

Designing a board doing the same as existing boards is wheel reinvention.

[rkonklex]

You're welcome!

The board by itself doesn't allow to use different clocks. It's 16MHz by default. I just switched the crystal oscillator in mine to 20MHz, since I had to replace the fried MCU anyways.

Regarding oversampling, it does actually both increase resolution and smooth out noise. It actually uses the noise to increase the resolution. See the whitepaper - fascinating stuff!

[Traumflug]

Regarding oversampling, it does actually both increase resolution and smooth out noise.

I think I do know how resolution enhancement by oversampling works, but your code doesn't do this. Resolution enhancement means that the 10-bit value is "enhanced" to 12 or more bits. This would require adjusting the ADC to Celsius lookup, because it's then no longer 10.0 fixed decimal, but 10.2 or 10.4. Reading a 10.2 value with the current code means to have a four times higher ADC reading.

Thanks for the insight about the Tronxy board. I'll keep both, but make 16 MHz the default.

[rkonklex]

Sure it does enhance the resolution. For example, for ADC_OVERSAMPLE_BITS=2, for a single analog read, first it generates additional 4 bits of data by collecting 16 samples into the accumulator. This is the oversampling stage. Then, it decimates the samples by discarding half of the additional bits of data, i.e. right-shifts the accumulator by 2 bits. In the end, by sacrificing speed and signal bandwidth by a factor of 16, we are left with 2 extra bits of resolution.

[Wurstnase]

You collecting 16 samples. Divide them by 4, and in the end again. So it is just a mean. You need a new temptable for oversampling. Or do we miss anything hidden?

[rkonklex]

Actually, best have a look at the code.

This technique actually needs a bit of white noise to work. It's probabilistic. Imagine the raw ADC provides a resolution of 5mv / step. The signal you're trying to measure is for example 13mV. The ADC will output readings oscillating between 2 (for 10mV) and 3 (for 15mV). If you take 16 samples in a short time, then you'll have more 3's than 2's, because the real value of 13mV is closer to 15mV. Now, you sum up all the 16 samples and that's the accumulator variable. If you divide it by 16, then you'd get an average that's pretty close to 13mV. But since we don't want to use floating point, then instead we divide it by 4, getting a 14.2 fixed-width fractional number.

No need for additional tables. A 16-bit accumulator variable is enough. ADC_OVERSAMPLE_BITS is how many extra bits of resolution we want. ADC_OVERSAMPLE_SAMPLES is how many samples are needed to get a single reading of increased resolution. It's 4^ADC_OVERSAMPLE_BITS, so for 2 bits we need 16 samples, for 3 bits - 64 samples, etc.

Also, have a look at the whitepaper AVR121: Enhancing ADC resolution by
oversampling.

[Wurstnase]

Yes, I see what you are doing.

You make an oversample in the beginning. But in the end you have just a mean, because all bits you take are gone.

So you take for example 4 ADC readings. Then you shift by 1. So theoretically you have a double oversample. But in the final end you shift again by 1. So you could also sample 4 ADCs and divide them by 4 (or shift by 2). That is a mean.

[rkonklex]

There's only a single shift. Unless I'm not seeing something... look at the line 92 - that should be the only shift.

[Wurstnase]

First shift
Second shift

[Wurstnase]

Your mean is just a little bit more hidden.

[rkonklex]

Oh, you're right! That's for the AD595 sensor, which I simply overlooked :) The code was meant for and tested for thermistors

[rkonklex]

I mean, originally it was
temp = (temp * 500L) >> 8;
Since I'm adding ADC_OVERSAMPLE_BITS, then I figured it should be shifted by (8 + ADC_OVERSAMPLE_BITS), instead of 8. So now it's
temp = (temp * 500L) >> (8 + ADC_OVERSAMPLE_BITS);
But that was just guessing.

[Traumflug]

Also, have a look at the whitepaper

Right. Nobody is questioning this application note. Resolution can be enhanced, at least in theory.

I just tried to run your code:

• adc_oversample_accum overflows with oversampling bits > 3, so this shouldn't be offered in Configtool
• config_wrapper.h isn't #included, so your code isn't compiled in, no matter how many oversampling bits are requested. This is why it appears to work, while it actually doesn't.

After fixing the latter I get ADC readings of 4092 with thermistor disconnected (should be 1023), which is clearly out of the thermistor table range.

If you move now the second shift to where it belongs, you drop the additional resolution and end up with averaging.

Also, these findings indicate that you just added code without investigating wether this additional code actually brings an enhancement. Not the ideal strategy. Additional code should improve things noticeably, else it's pointless to keep it. @phord did this nicely in #208, see the two graphs in the opening post. The point should not be to add code, but to improve printing results.

[rkonklex]

First, config_wrapper.h is #included in analog.c, which in turn includes analog-avr.c. And analog-avr.c seems to do anything only when included from analog.c. So this is fine.

The second shift shouldn't be there. The code was intended for thermistors, and obviously the oversampled formula for AD595 is wrong.

Reading this large is what was intended. You maxed out at 4092, which means you enabled oversampling for 2 extra bits of resolution. That's what it is. Instead of a value 0-1023 you get one from the 0-4092 range - a 12-bit value, instead of a 10-bit value. You need to adjust thermistor tables - that's what the maxAdc commit was for.

The improvement was enhancing the resolution. However, I must admit that @phord's patch to table generation algorithm made this much less needed. Even without oversampling, now there's 2-3 ADC steps per degree in the most interresting range of 160 - 290 Celsius, which is more than enough.

[Traumflug]

You need to adjust thermistor tables - that's what the maxAdc commit was for.

Now we get closer :-) So there's a mechanism missing for raising maxAdc depending on oversampling at table creation time.

[rkonklex]

You could say so. I meant to change the maxAdc setting each time in configtool's options, but that's a poor solution, obviously. I'll try and add adjusting maxAdc based on oversampling selection.

[rkonklex]

Actually, the formula for AD595 was correct - and oversampling would improve its accuracy. At first oversampling seems useless for this formula, but let's not forget we're dealing with integer arithmetic here.

The original formula is (temp * 500L) >> 8. Let's assume a 10-bit ADC range of 0 - 1023. For ADC=131, the formula gives 255, which is actually 63.75 deg. C, since it's 14.2 fixed-point. For ADC=132 it's 64.25 deg. C. For non-oversampled 10-bit ADC, there are no steps between 131 and 132, and no possibility to get a 64.00 deg. C reading.

Now, with 2 bit oversampling, the ADC becomes 12-bit with a range of 0 - 4092. The value 131 (10b) is now read as 524 (12b), and 132 (10b) becomes 528 (12b). Inbetween there are now three additional values: 525, 526, 527. The oversampled formula is (temp * 500L) >> (8 + ADC_OVERSAMPLE_BITS), which becomes (temp * 500L) >> 10 in our case, since we're oversampling 2 bits. The temperatures are:
ADC 524 (12b) -> 63.75 deg. C
ADC 525 (12b) -> 64.00 deg. C
ADC 526 (12b) -> 64.00 deg. C
ADC 527 (12b) -> 64.25 deg. C
ADC 528 (12b) -> 64.25 deg. C

Which means that oversampling enables reading of 64.00 deg. C, which was not possible with the default 10-bit, non-oversampled ADC. In the case of AD595 1-bit oversampling would have been enough. For thermistors, 2-bit oversampling is optimal. In either case, more than that and the additional accuracy is truncated by the 14.2 format, in which temperatures are stored.

[Wurstnase]

Right, this will work for your AD595, but on other thermistors you can't simply use the result of the oversampled ADC. You need also a 12bit temperature-table.

[rkonklex]

Sure, you need 12-bit temperature tables. I made a quick-and-dirty fix for that, and I'm using it myself. I'll refactor it into a proper solution integrated into the configtool.

[Wurstnase]

This Tronxy-stuff is a Sanguinolulu v1.2?!? It has the same pins etc.
Also the Tronxy-Printer? Do we need it? It has a buffer of 96?

http://forums.reprap.org/read.php?146,705869,705869#msg-705869
http://forums.reprap.org/read.php?147,33082,705240#msg-705240

[Traumflug]

The board in the picture looks much more like a Melzi, which runs an ATmega1284p. Not exactly one, because there are pluggable motor headers, not screw terminals. Sanguino usually runs the same MCU in the DIP version (or a 644p).

For an Arduino Uno a movebuffer size of 96 is way too much, 10 is about the limit. An '1284p can deal with about 64.

The most buggy part here is that buffer size isn't a board, but a printer setting :-)

[Wurstnase]

Yes, maybe a Melzi. I only diff the board.sanguinolulu v1.2 with tronxy. And there is nothing different except a fan.

[rkonklex]

Hi!
Tronxy was my addition - I own one. It has a Melzi-derived board with an ATmega1284p and it supports a MOSFET-controlled cooling fan. I think that was the main reason for adding another configuration.

Movebuffer is way too much, I agree. The MCU has 16 kilobytes of RAM, so in theory it could handle a couple dozen of movebuffer entries. To me, the best value is 10-16 entries, depending on how much latency you can tolerate.

[Wurstnase]

Nothing against a new board and printer layout. Maybe it's better to rename this to Melzi or something like this. So others can use it for their printer which is not called 'Tronxy'.

So it is a Melzi?