Help with DDS and PWM

Go To Last Post
86 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi
I need to generate a sinusoidal PWM signal, with variable frequency with atmega 328P/328.I generated for that, a series of arrays. The problem is that every one of the array has a different number of elements, with the bigger having 3200+ elemtents (just one) and the tiniest, just 14 elements. So, probably you can be agree with me that is a bad idea to place them in a nXm matrix, because the enormous quantity of padding.

 

Declaring every one of those arrays alone with its own label isn't a Good idea (at least for me), because I had to reference them explicitly, being 255 arrays. So I think about reading them as a list, but still being an array, a big array with all arrays inside in the same row, for that I included in the array the large of every one of the arrays at the start of every array data. However, I still had the following problem: Compiler says that this array is too big. Is no memory related, because I tested, compiling the first case with all arrays separated and compiler did it succesfully storing all that data in flash. Probably is sanity limit related. The resulting array has 19929 elements of unsigned 16 bits type.

I included both versions so you can check.

How I can do this? there's a workaround for array size limits? its a Good idea to link raw data into executable image?

thanks

 

Attachment(s): 

Last Edited: Tue. Jul 9, 2019 - 03:24 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You need to put the tables in flash. Const doesnt do this for you with avr-gcc. Read up on _flash that puts it into flash.

Next, why such a large table? Use a technique like DDS to use a smaller table. Usually 256 entries suffices.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks but, my objective is not to generate the sine waveform with microcontroller  itself, I need those pulses intact, to feed driver and make a power stage to do it (make the transistor driver to switch a couple of power transistors). The transistor driver does not accept analog input. In fact, in the rest of the program I put those numbers in the PWM generator, not in a I/O port to interface a DAC. If you take a look, those numbers are limited to 1023, because I'm using the 10-bit PWM.

The problem is't that data can't fit into mcu, it fitted when I declared arrays separately. The problem is the compiler that can't accept such array by the default way.

Last Edited: Sat. Jul 6, 2019 - 01:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There was no mention of a DAC. Again, use the DDS technique to interpolate a sine table of 256 entries and output to pwm. You can get sub Hz resolution if you want. You can scale the output of the DDS to get variable go,tage. This is how its done in VFDs.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, now I'm on a real computer - my ipad was going flat.

 

 

You want something like:

 

uint32_t phase_acc = 0,phase_inc;

const __flash uint16_t sine_tab[256] = {.....};

// phase_inc has the value that translates to the frequency you want.
// one cycle is when phase_acc overflows. So if you want 50 cycles,
// phase_inc = 2^32/50
// for the actual frequency, you need to factor in your pwm freq

// run this every pwm cycle.
pwm = sine_tab[phase_acc>>24];
      phase_acc += phase_inc;

Using a 32bit phase_acc means you get very fine frequency resolution. You could use 16bit, but you'll need to crunch the numbers to figure out if you'll get the results you want.

 

 

Some discussion of __flash:

https://www.avrfreaks.net/forum/...

 

Last Edited: Sat. Jul 6, 2019 - 01:46 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>The resulting array has 19929 elements of unsigned 16 bits type.

 

19929 * 2 bytes each = 39858 bytes

your mega328 has 32k of flash ->   32K < 39858 = no fit

 

I would guess your flock of tables only 'worked' because any unused was simply being optimized away (they were being put in ram also, so probably all optimized away unless the smaller tables were used).

Your big table was marked const, but for an avr I'm quite sure you need __attribute((progmem)) (or some simpler equivalent), so even in that case was set to go to ram.

(the avr-0/1 series can use const and the data will go in flash, because flash space is addressable via normal instructions)

 

In any case 32K seems to be the limit for array sizes in avr- uint8_t[32k], uint16_t[16k], uint32_t[8k], whatever gets you to 32k, and for a 328 you will never get to test the limit.

 

Last Edited: Sat. Jul 6, 2019 - 04:05 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The compiler limits all arrays to maximum 32768 bytes regardless of type.
Just split into two arrays.
.
You might get awy with putting two arrays in a packed struct. Then use pgmspace.h functions to access with a single "index".
.
I doubt if there is any point in using more than 256 or 1024 steps in PWM.
.
David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would also go with a DDS, (it's just an other way to pick the needed value), and if scaled correct you can still get more values than in the table (just some will be the same, as in your big tables.). 

 

Which update speed do you need ? and how does it match the PWM speed? (no need to make the calcs way better than the PWM can output) 

will you also need a gain calculation ?  

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Otherwise, the technique for storing data in memory with varying length "rows" is to store an array of pointers to arrays.

 

In AVR, you put this in __flash unless you need to change it.

The largest known prime number: 282589933-1

It's easy to stop breaking the 10th commandment! Break the 8th instead. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry to be a pedant but can I point out that the issue is more about defining than declaring (thread title) ;-)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You should put all your array data into a serial EEPROM.  In your Program flash have a table of 256 addresses: each address holds the start of the Sine's variable-length array of data bytes.   To "do" sine(147), get the address from flash of the beginning of the data that sine[147] is stored in the Serial EEPROM. 

  Now have either a second table for the length of each of the 255 sine[] entries, or have each sine[] array of data end with the byte value of 0xff (which is a sentinel value that the actual data will never be.)

 

  This data doesn't make any sense to me.  If you are using fixed time intervals for the PWM, then each array with same number of elements is going to have the same frequency.  The differences in the actual data values will appear in the final signal as harmonic distortion.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Looks pretty simple. But what will happen with carry there?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

Thanks but, my objective is not to generate the sine waveform with microcontroller  itself, I need those pulses intact, to feed driver and make a power stage to do it (make the transistor driver to switch a couple of power transistors). The transistor driver does not accept analog input. In fact, in the rest of the program I put those numbers in the PWM generator, not in a I/O port to interface a DAC. If you take a look, those numbers are limited to 1023, because I'm using the 10-bit PWM.

The problem is't that data can't fit into mcu, it fitted when I declared arrays separately. The problem is the compiler that can't accept such array by the default way.

As has been mentioned the problem IS that data can't fit into mcu.

Did you look at the suggested DDS ?  It is actually quite close to what you are already doing via tables.

Unless there is something special about your data tables, the usual approach is to define the largest one, (here 3216 elements = DDS:+1) and for the 1608 case, your DDS:+2 adder reads every second element, for the 1072 case, your DDS:+3 adder reads every 3rd element etc 

The case of 644 becomes interesting, as that is almost /5, a DDS:+5 adder will add 5 to the index every time, and  so divides by 643.2, that means 4 scans of 5 it reads 643 values and the 5th time it reads 644 points.

 

If you really do need lots of tables because of complex PWM needs, (eg fine control of voltage) you may have to look at external tables in Flash. 

With Flash, the possible size explodes, and you can now store multiple whole cycles of tables, so your 644 case can store 5 cycles in a 3216 array, and avoids the rounding error your 644 gives.

Even if all your tables are 3126 sized, that 1.640MBytes, which is < 50c/100pcs

 

You will need to use a portion of MCU time to read the flash for every PWM cycle, - a re-index of a SPI array is 48 SPI clocks, and a read-next is 16 SPI clocks.

SPI usually can be SysCLK/2 max, so you need 96 SysCLKs to read a re-index, or ballpark 10% of CPU loading. 

You might explore a `read hybrid`, where 8 bits read via HW-SPI and 2 more bits clock in SW, which now packs 10b values into the Flash

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Who-me wrote:

ColdfireMC wrote:

Thanks but, my objective is not to generate the sine waveform with microcontroller  itself, I need those pulses intact, to feed driver and make a power stage to do it (make the transistor driver to switch a couple of power transistors). The transistor driver does not accept analog input. In fact, in the rest of the program I put those numbers in the PWM generator, not in a I/O port to interface a DAC. If you take a look, those numbers are limited to 1023, because I'm using the 10-bit PWM.

The problem is't that data can't fit into mcu, it fitted when I declared arrays separately. The problem is the compiler that can't accept such array by the default way.

As has been mentioned the problem IS that data can't fit into mcu.

Did you look at the suggested DDS ?  It is actually quite close to what you are already doing via tables.

Unless there is something special about your data tables, the usual approach is to define the largest one, (here 3216 elements = DDS:+1) and for the 1608 case, your DDS:+2 adder reads every second element, for the 1072 case, your DDS:+3 adder reads every 3rd element etc 

The case of 644 becomes interesting, as that is almost /5, a DDS:+5 adder will add 5 to the index every time, and  so divides by 643.2, that means 4 scans of 5 it reads 643 values and the 5th time it reads 644 points.

 

If you really do need lots of tables because of complex PWM needs, (eg fine control of voltage) you may have to look at external tables in Flash. 

With Flash, the possible size explodes, and you can now store multiple whole cycles of tables, so your 644 case can store 5 cycles in a 3216 array, and avoids the rounding error your 644 gives.

Even if all your tables are 3126 sized, that 1.640MBytes, which is < 50c/100pcs

 

You will need to use a portion of MCU time to read the flash for every PWM cycle, - a re-index of a SPI array is 48 SPI clocks, and a read-next is 16 SPI clocks.

SPI usually can be SysCLK/2 max, so you need 96 SysCLKs to read a re-index, or ballpark 10% of CPU loading. 

You might explore a `read hybrid`, where 8 bits read via HW-SPI and 2 more bits clock in SW, which now packs 10b values into the Flash

 

 

 

 

 

Kartman wrote:

Ok, now I'm on a real computer - my ipad was going flat.

 

 

You want something like:

 

uint32_t phase_acc = 0,phase_inc;

const __flash uint16_t sine_tab[256] = {.....};

// phase_inc has the value that translates to the frequency you want.
// one cycle is when phase_acc overflows. So if you want 50 cycles,
// phase_inc = 2^32/50
// for the actual frequency, you need to factor in your pwm freq

// run this every pwm cycle.
pwm = sine_tab[phase_acc>>24];
      phase_acc += phase_inc;

Using a 32bit phase_acc means you get very fine frequency resolution. You could use 16bit, but you'll need to crunch the numbers to figure out if you'll get the results you want.

 

 

Some discussion of __flash:

https://www.avrfreaks.net/forum/...

 


Thanks for your wise advice guys, really.
I looked better at Direct Synthesis, read a couple of tutorials and a book chapter, my question is just: If I program it directly in C, what I have to do to get and use the carry to estimate phase error, or if it's just not needed in this case(or i'm just plain wrong)? The second is, If I not load the huge tables, I have to load just one sine table only, so how much I can gain in quality if I put there a 10 bit sine table instead of 8? I set the 10bit pwm because of it lowers the switching frequency more than 8-bit, and gives more possible levels. I need a sub 10Khz switching frequency to avoid greater switching losses.

 

 

I'm going to change my ISR to set the accumulator and read the table and I then post it here.

Thanks

Last Edited: Sun. Jul 7, 2019 - 12:37 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It is simple. No need to be concerned about carry - phase_acc just rolls around. The beauty of fixed precision binary math! Try it and see. Use a simple rc filter on your pwm output and look at it on an oscilloscope (no scope? Use your pc soundcard as a scope). When you’re happy with the operation, then hook up your high power driver.

I’m surprised you haven't come across this technique before. I think Microchip have some app notes on VSDs with their pic micros.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You snuck in behind me whilst i was typing! I made my sine table 16 bit but with 256 samples. For 10bit, just scale the values in the sine table accordingly. You still have 256 samples.
The average VFD uses around 4kHz switching frequency.
If you perform the test i suggested, then you’ll see that the ‘sine wave’ gets more ragged as you increase the output frequency. IE the harmonics increase. This is usually not a concern as the motor inductance smooths the waveform.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
It is simple. No need to be concerned about carry - phase_acc just rolls around. The beauty of fixed precision binary math! Try it and see. Use a simple rc filter on your pwm output and look at it on an oscilloscope (no scope? Use your pc soundcard as a scope). When you’re happy with the operation, then hook up your high power driver. I’m surprised you haven't come across this technique before. I think Microchip have some app notes on VSDs with their pic micros.

That is the setup that I have now, a Scope, an 74LS buffer and a khz range LC filter to check if output is correct. The original tables, at least some entries I tested, work, but need to be able to change frequency so the tables were just really a weird "excess of coffee" idea.
Sorry by just uploading a photo, I don't have a GPIB interface for this scope yet .

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

I looked better at Direct Synthesis, read a couple of tutorials and a book chapter, my question is just: If I program it directly in C, what I have to do to get and use the carry to estimate phase error, or if it's just not needed in this case(or i'm just plain wrong)? The second is, If I not load the huge tables, I have to load just one sine table only, so how much I can gain in quality if I put there a 10 bit sine table instead of 8? I set the 10bit pwm because of it lowers the switching frequency more than 8-bit, and gives more possible levels. I need a sub 10Khz switching frequency to avoid greater switching losses.

Certainly 10b is one quarter the granularity of 8b, per sample.  If you need sub 10kHz, the 10b comes almost for free.

As to if that matters, it depends what this is actually used for ?

You might want 10b (or even higher) tables if you also scale or multiply to vary modulation depth

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Who-me wrote:

ColdfireMC wrote:

I looked better at Direct Synthesis, read a couple of tutorials and a book chapter, my question is just: If I program it directly in C, what I have to do to get and use the carry to estimate phase error, or if it's just not needed in this case(or i'm just plain wrong)? The second is, If I not load the huge tables, I have to load just one sine table only, so how much I can gain in quality if I put there a 10 bit sine table instead of 8? I set the 10bit pwm because of it lowers the switching frequency more than 8-bit, and gives more possible levels. I need a sub 10Khz switching frequency to avoid greater switching losses.

Certainly 10b is one quarter the granularity of 8b, per sample.  If you need sub 10kHz, the 10b comes almost for free.

As to if that matters, it depends what this is actually used for ?

You might want 10b (or even higher) tables if you also scale or multiply to vary modulation depth


I want to place a gain to have some degree of Amplitude control, but still have to test the minimal 8bit table.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Looks like a TDS210. Adding GPIB these days would cost more than the scope is worth! You can buy a Rigol that has USB for around USD $500. Regardless, the picture is fine.

 

Why would you add a 74LS buffer? The output of the AVR is probably more symmetrical and has better drive. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

Looks like a TDS210. Adding GPIB these days would cost more than the scope is worth! You can buy a Rigol that has USB for around USD $500. Regardless, the picture is fine.

 

Why would you add a 74LS buffer? The output of the AVR is probably more symmetrical and has better drive. 

To isolate the MCU output from the filter to be able to read unfiltered signal, and also the filtered at the same time. Just test setup. The final will have a half-bridge driver like IRF2103 or simmilar.
precisely: a tek 210. it has the GPIB+serial interface installed, but I don't have a gpib to usb or something like that to take decent shots. I used serial interface, raw and visa time ago, but has lots of silly bugs and fixing them was a nightmare.

Last Edited: Sun. Jul 7, 2019 - 02:08 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OP, it looks like your tables are doing a full 360 degrees of sine data (correct me if I'm wrong).  Do you realize that all you really need is 90 degrees of data and a bit of math?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Whilst you really only need 90 degrees, The micro is probably pushed doing the DDS calc in real time, so adding extra work to figure out the quadrant doesn't help. The usual tradeoff of size,speed etc.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm going to use DDS, so table will be considerably smaller. I used 180 degrees in the original tables

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:
I'm going to use DDS, so table will be considerably smaller. I used 180 degrees in the original tables

 

Did you use 1st and 2nd quadrants? It would probably be faster to use 1st and 3rd and change direction than 1st and 2nd and change sign.

 

That is: 1st in forward sense = 1st

1st reverse = 2nd

3rd forward = 3rd

3rd reverse = 4th

 

This way, for example using the code in post #5, you just need to do adjustments at the boundaries, but other than that the code is the same.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi guys. I think that I did it. I only have to implement buttons to control this, but it worked. Some details:

 

The phase accumulator doesn't work exactly like intended when using the PWM counter compare interrupt. The accumulator fills up just to the pwm speed. Still has pretty Good results

hence Shifting wasn't needed

Also, I managed to load a 10-bit table, and worked around the overflow with a little comparison.

So, here's the code.
 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
#include <math.h>
//volatile bool back_forth=0;
//volatile int reg;
volatile int sample_index;
volatile uint16_t phase_acc = 0;
volatile uint16_t phase_inc = 16;
const uint16_t __flash sine[1024]={511, 514, 517, 520, 524, 527, 530, 533, 536, 539, 542, 546, 549, 552, 555, 558, 561, 564, 567, 571, 574, 577, 580, 583, 586, 589, 592, 595, 599, 602, 605, 608, 611, 614, 617, 620, 623, 626, 629, 632, 635, 638, 641, 645, 648, 651, 654, 657, 660, 663, 666, 669, 672, 675, 678, 681, 683, 686, 689, 692, 695, 698, 701, 704, 707, 710, 713, 716, 718, 721, 724, 727, 730, 733, 736, 738, 741, 744, 747, 750, 752, 755, 758, 761, 763, 766, 769, 772, 774, 777, 780, 782, 785, 788, 790, 793, 795, 798, 801, 803, 806, 808, 811, 813, 816, 819, 821, 824, 826, 828, 831, 833, 836, 838, 841, 843, 845, 848, 850, 853, 855, 857, 859, 862, 864, 866, 869, 871, 873, 875, 877, 880, 882, 884, 886, 888, 890, 892, 895, 897, 899, 901, 903, 905, 907, 909, 911, 913, 915, 917, 918, 920, 922, 924, 926, 928, 930, 931, 933, 935, 937, 938, 940, 942, 944, 945, 947, 949, 950, 952, 953, 955, 956, 958, 960, 961, 963, 964, 965, 967, 968, 970, 971, 972, 974, 975, 976, 978, 979, 980, 982, 983, 984, 985, 986, 988, 989, 990, 991, 992, 993, 994, 995, 996, 997, 998, 999, 1000, 1001, 1002, 1003, 1004, 1004, 1005, 1006, 1007, 1008, 1008, 1009, 1010, 1011, 1011, 1012, 1013, 1013, 1014, 1014, 1015, 1015, 1016, 1016, 1017, 1017, 1018, 1018, 1019, 1019, 1020, 1020, 1020, 1021, 1021, 1021, 1021, 1022, 1022, 1022, 1022, 1022, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1022, 1022, 1022, 1022, 1022, 1021, 1021, 1021, 1021, 1020, 1020, 1020, 1019, 1019, 1018, 1018, 1017, 1017, 1016, 1016, 1015, 1015, 1014, 1014, 1013, 1013, 1012, 1011, 1011, 1010, 1009, 1008, 1008, 1007, 1006, 1005, 1004, 1004, 1003, 1002, 1001, 1000, 999, 998, 997, 996, 995, 994, 993, 992, 991, 990, 989, 988, 986, 985, 984, 983, 982, 980, 979, 978, 976, 975, 974, 972, 971, 970, 968, 967, 965, 964, 963, 961, 960, 958, 956, 955, 953, 952, 950, 949, 947, 945, 944, 942, 940, 938, 937, 935, 933, 931, 930, 928, 926, 924, 922, 920, 918, 917, 915, 913, 911, 909, 907, 905, 903, 901, 899, 897, 895, 892, 890, 888, 886, 884, 882, 880, 877, 875, 873, 871, 869, 866, 864, 862, 859, 857, 855, 853, 850, 848, 845, 843, 841, 838, 836, 833, 831, 828, 826, 824, 821, 819, 816, 813, 811, 808, 806, 803, 801, 798, 795, 793, 790, 788, 785, 782, 780, 777, 774, 772, 769, 766, 763, 761, 758, 755, 752, 750, 747, 744, 741, 738, 736, 733, 730, 727, 724, 721, 718, 716, 713, 710, 707, 704, 701, 698, 695, 692, 689, 686, 683, 681, 678, 675, 672, 669, 666, 663, 660, 657, 654, 651, 648, 645, 641, 638, 635, 632, 629, 626, 623, 620, 617, 614, 611, 608, 605, 602, 599, 595, 592, 589, 586, 583, 580, 577, 574, 571, 567, 564, 561, 558, 555, 552, 549, 546, 542, 539, 536, 533, 530, 527, 524, 520, 517, 514, 512, 509, 506, 503, 499, 496, 493, 490, 487, 484, 481, 477, 474, 471, 468, 465, 462, 459, 456, 452, 449, 446, 443, 440, 437, 434, 431, 428, 424, 421, 418, 415, 412, 409, 406, 403, 400, 397, 394, 391, 388, 385, 382, 378, 375, 372, 369, 366, 363, 360, 357, 354, 351, 348, 345, 342, 340, 337, 334, 331, 328, 325, 322, 319, 316, 313, 310, 307, 305, 302, 299, 296, 293, 290, 287, 285, 282, 279, 276, 273, 271, 268, 265, 262, 260, 257, 254, 251, 249, 246, 243, 241, 238, 235, 233, 230, 228, 225, 222, 220, 217, 215, 212, 210, 207, 204, 202, 199, 197, 195, 192, 190, 187, 185, 182, 180, 178, 175, 173, 170, 168, 166, 164, 161, 159, 157, 154, 152, 150, 148, 146, 143, 141, 139, 137, 135, 133, 131, 128, 126, 124, 122, 120, 118, 116, 114, 112, 110, 108, 106, 105, 103, 101, 99, 97, 95, 93, 92, 90, 88, 86, 85, 83, 81, 79, 78, 76, 74, 73, 71, 70, 68, 67, 65, 63, 62, 60, 59, 58, 56, 55, 53, 52, 51, 49, 48, 47, 45, 44, 43, 41, 40, 39, 38, 37, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 19, 18, 17, 16, 15, 15, 14, 13, 12, 12, 11, 10, 10, 9, 9, 8, 8, 7, 7, 6, 6, 5, 5, 4, 4, 3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10, 11, 12, 12, 13, 14, 15, 15, 16, 17, 18, 19, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 37, 38, 39, 40, 41, 43, 44, 45, 47, 48, 49, 51, 52, 53, 55, 56, 58, 59, 60, 62, 63, 65, 67, 68, 70, 71, 73, 74, 76, 78, 79, 81, 83, 85, 86, 88, 90, 92, 93, 95, 97, 99, 101, 103, 105, 106, 108, 110, 112, 114, 116, 118, 120, 122, 124, 126, 128, 131, 133, 135, 137, 139, 141, 143, 146, 148, 150, 152, 154, 157, 159, 161, 164, 166, 168, 170, 173, 175, 178, 180, 182, 185, 187, 190, 192, 195, 197, 199, 202, 204, 207, 210, 212, 215, 217, 220, 222, 225, 228, 230, 233, 235, 238, 241, 243, 246, 249, 251, 254, 257, 260, 262, 265, 268, 271, 273, 276, 279, 282, 285, 287, 290, 293, 296, 299, 302, 305, 307, 310, 313, 316, 319, 322, 325, 328, 331, 334, 337, 340, 342, 345, 348, 351, 354, 357, 360, 363, 366, 369, 372, 375, 378, 382, 385, 388, 391, 394, 397, 400, 403, 406, 409, 412, 415, 418, 421, 424, 428, 431, 434, 437, 440, 443, 446, 449, 452, 456, 459, 462, 465, 468, 471, 474, 477, 481, 484, 487, 490, 493, 496, 499, 503, 506, 509};
void pwm_setup(void);

int main(void)
{
	OCR1A=0;
	DDRD|=0xFF;
	DDRB|=0xFF;
	TIMSK1=(1<<OCIE1A);
	OCR1A=1;
	sample_index=0;
	//reg=1;
	pwm_setup();
	sei();
	while (1)

	{

	}
	return 0;
}

ISR(TIMER1_COMPA_vect)
{
	unsigned int reg=0;
	uint16_t index;
	float roundreg;

	if (phase_acc<=1023)
	{

	index=phase_acc;
	reg=OCR1A;
	roundreg=sine
; reg = round(roundreg/1.1); phase_acc += phase_inc; } else { phase_acc=0; } OCR1A=reg; } void pwm_setup(void) { TCCR1A |= (1 << COM1A1); TCCR1A |= (1 << WGM11) | (1 << WGM10); TCCR1B |= (0 << CS10) | (1 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12); }

Also the Little boy working 

What you think about?

Attachment(s): 

Last Edited: Mon. Jul 8, 2019 - 09:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I gave you a worked solution and you screw it up! There was a specific reason I made the sine table 256 entries long. Because you changed it, you had to fudge up the code to make it sort of work. I’d suggest you do exactly what i suggested and see how that works. Not only is it simpler, it will work as intended.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
I gave you a worked solution and you screw it up! There was a specific reason I made the sine table 256 entries long. Because you changed it, you had to fudge up the code to make it sort of work. I’d suggest you do exactly what i suggested and see how that works. Not only is it simpler, it will work as intended.

I think that did it exactly as you posted and this was the result

 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
volatile uint32_t phase_acc = 0;
volatile uint32_t phase_inc = (2^32)/50;

const __flash uint8_t sine[256]={127, 130, 133, 136, 140, 143, 146, 149, 152, 155, 158, 161, 164, 167, 170, 173, 176, 179, 182, 185, 187, 190, 193, 195, 198, 201, 203, 206, 208, 211, 213, 215, 218, 220, 222, 224, 226, 228, 230, 232, 233, 235, 237, 238, 240, 241, 243, 244, 245, 246, 248, 249, 249, 250, 251, 252, 253, 253, 254, 254, 254, 255, 255, 255, 255, 255, 255, 255, 254, 254, 254, 253, 253, 252, 251, 250, 249, 249, 248, 246, 245, 244, 243, 241, 240, 238, 237, 235, 233, 232, 230, 228, 226, 224, 222, 220, 218, 215, 213, 211, 208, 206, 203, 201, 198, 195, 193, 190, 187, 185, 182, 179, 176, 173, 170, 167, 164, 161, 158, 155, 152, 149, 146, 143, 140, 136, 133, 130, 128, 125, 122, 119, 115, 112, 109, 106, 103, 100, 97, 94, 91, 88, 85, 82, 79, 76, 73, 70, 68, 65, 62, 60, 57, 54, 52, 49, 47, 44, 42, 40, 37, 35, 33, 31, 29, 27, 25, 23, 22, 20, 18, 17, 15, 14, 12, 11, 10, 9, 7, 6, 6, 5, 4, 3, 2, 2, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 3, 4, 5, 6, 6, 7, 9, 10, 11, 12, 14, 15, 17, 18, 20, 22, 23, 25, 27, 29, 31, 33, 35, 37, 40, 42, 44, 47, 49, 52, 54, 57, 60, 62, 65, 68, 70, 73, 76, 79, 82, 85, 88, 91, 94, 97, 100, 103, 106, 109, 112, 115, 119, 122, 125};
	void pwm_setup(void);

int main(void)
{
	OCR1A=0;
	DDRD|=0xFF;
	DDRB|=0xFF;
	TIMSK1=(1<<OCIE1A);
	OCR1A=1;
	sample_index=0;
	//reg=1;
	pwm_setup();
	sei();
	while (1)

	{

	}
	return 0;
}

ISR(TIMER1_COMPA_vect)
{
	unsigned int reg;
	//uint8_t index;

	//index=phase_acc;
	reg=OCR1A;
	//if (phase_acc<=255)
	//{
	reg = sine[phase_acc>>24];
	phase_acc += phase_inc;
	//}
	//else
	//{
	//	phase_acc=0;
	//}
	OCR1A=reg;

}

void pwm_setup(void)
{
	TCCR1A |= (1 << COM1A1);
	TCCR1A |= (0 << WGM11) | (1 << WGM10);
	TCCR1B |= (0 << CS10) | (1 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

 

If I do it exactly as you posted, the shift erases the phase accumulator and just loads the first value of the table if I put that sentence in the ISR. The ISR is too slow to fill it. Probably I'm missing something.

 

have I to put the accumulator outside ISR and then put that value in the Comparation register every PWM cycle? How I know that the ISR Will hit the accumulator in the right value of phase?

 

Last Edited: Tue. Jul 9, 2019 - 12:10 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, I had my 'Moloko Plus' to sharpen me up. And I'm on a real computer again.

 

Seems there's some subtle details hidden in the apparent simplicity of what I proposed.

 

Since I used a 32bit variable for the phase accumulator, There is an implied radix point between bits 24 and 25. This means the integer part is 8 bits and the fractional part is 24 bits - 2^24 or over 7 decimal digits. Your concept of 'carry' is implicit in the 32 bit addition. If you used a 16 bit phase accumulator, then the fraction would only be 2^8 or just over 2 decimal digits of fraction. 

 

 

The next subtle thing is the use of a sine table of 256 entries. The index comes out to 8 bits. We're using an 8 bit processor. When I do the phase_acc>>24, effectively I'm extracting the integer part of the phase accumulator and using that to index the sine table. Here's the trick - the avr can only do a bit shift of 1 bit on an 8 bit variable in 1 instruction. So to do a sift of 24bits on a 32bit variable, this is going to take 24 *4 shifts along with reading/writing the intermediate values. This is slow. However, the compiler figures out that it on a byte boundary and simply extracts the byte it wants - no shifting. If you used a 1024 entry sine table, then you would need to do expensive shifts. 

 

If you're the inquisitive type (you should be), you can measure the execution times of your code and my code by toggling a port bit before and after the code. The scope can measure the time. You can also look at the assembler code generated by the compiler to see what is more efficient.

 

So, whilst the code I posted was simple - there's some tricks embedded into it. Ignore at your own peril!

 

Another thing to note - floating point on an AVR is slow - you really never want to do this in an ISR.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You've sneeked in whilst I was typing - 

const __flash uint8_t sine - this is not what I wrote!
const __flash uint16_t sine_tab[256] = {.....}; 

You wanted 10 bit pwm - the sine values therefore need to be between 0..1023. You need 256 of them.

 

you might want to check this value:

 

volatile uint32_t phase_inc = (2^32)/50;

The compiler might have trouble calculating this value. Do it by hand first up.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

volatile uint32_t phase_inc = (2^32)/50;

The compiler might have trouble calculating this value. Do it by hand first up.

Methinks the OP expects C to treat ^ as a means of computing powers... :-)

 

... but in all fairness, that's how you expressed it in the comments of your code in #5.

 

To the OP, I'd try:

 

volatile uint32_t phase_inc = (1ULL << 32) / 50;

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Tue. Jul 9, 2019 - 12:11 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
volatile uint32_t phase_acc = 0;
volatile uint32_t phase_inc =255*2000;
volatile unsigned int reg;
const __flash uint8_t sine[256]={127, 130, 133, 136, 140, 143, 146, 149, 152, 155, 158, 161, 164, 167, 170, 173, 176, 179, 182, 185, 187, 190, 193, 195, 198, 201, 203, 206, 208, 211, 213, 215, 218, 220, 222, 224, 226, 228, 230, 232, 233, 235, 237, 238, 240, 241, 243, 244, 245, 246, 248, 249, 249, 250, 251, 252, 253, 253, 254, 254, 254, 255, 255, 255, 255, 255, 255, 255, 254, 254, 254, 253, 253, 252, 251, 250, 249, 249, 248, 246, 245, 244, 243, 241, 240, 238, 237, 235, 233, 232, 230, 228, 226, 224, 222, 220, 218, 215, 213, 211, 208, 206, 203, 201, 198, 195, 193, 190, 187, 185, 182, 179, 176, 173, 170, 167, 164, 161, 158, 155, 152, 149, 146, 143, 140, 136, 133, 130, 128, 125, 122, 119, 115, 112, 109, 106, 103, 100, 97, 94, 91, 88, 85, 82, 79, 76, 73, 70, 68, 65, 62, 60, 57, 54, 52, 49, 47, 44, 42, 40, 37, 35, 33, 31, 29, 27, 25, 23, 22, 20, 18, 17, 15, 14, 12, 11, 10, 9, 7, 6, 6, 5, 4, 3, 2, 2, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 3, 4, 5, 6, 6, 7, 9, 10, 11, 12, 14, 15, 17, 18, 20, 22, 23, 25, 27, 29, 31, 33, 35, 37, 40, 42, 44, 47, 49, 52, 54, 57, 60, 62, 65, 68, 70, 73, 76, 79, 82, 85, 88, 91, 94, 97, 100, 103, 106, 109, 112, 115, 119, 122, 125};
void pwm_setup(void);

int main(void)
{
	OCR1A=0;
	DDRD|=0xFF;
	DDRB|=0xFF;
	TIMSK1=(1<<OCIE1A);
	OCR1A=1;
	//sample_index=0;
	//reg=1;
	pwm_setup();
	sei();
	while (1)

	{

		phase_acc += phase_inc;

	}
	return 0;
}

ISR(TIMER1_COMPA_vect)
{

	//uint8_t index;

	//index=phase_acc;
	//reg=OCR1A;
	//if (phase_acc<=255)
	//{
	//reg = sine[phase_acc>>24];
	//phase_acc += phase_inc;
	//}
	//else
	//{
	//	phase_acc=0;
	//}
	reg = sine[phase_acc>>24];
	OCR1A=reg;

}

void pwm_setup(void)
{
	TCCR1A |= (1 << COM1A1);
	TCCR1A |= (0 << WGM11) | (1 << WGM10);
	TCCR1B |= (1 << CS10) | (0 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

Ok, I'm a little dumb, but appears that now is working as intended, 8bit version (256 values, 256 entries)…

 

Kartman wrote:

The next subtle thing is the use of a sine table of 256 entries. The index comes out to 8 bits. We're using an 8 bit processor. When I do the phase_acc>>24, effectively I'm extracting the integer part of the phase accumulator and using that to index the sine table. Here's the trick - the avr can only do a bit shift of 1 bit on an 8 bit variable in 1 instruction. So to do a sift of 24bits on a 32bit variable, this is going to take 24 *4 shifts along with reading/writing the intermediate values. This is slow. However, the compiler figures out that it on a byte boundary and simply extracts the byte it wants - no shifting. If you used a 1024 entry sine table, then you would need to do expensive shifts. 


That explains a lot: I wrote also a 10bit version, with 1024 entries and 1024 values, and just "theoritcally Works" but waveform has some jitter. The problem is precisely that: The types are not aligned, so the shift is expensive and causes jitter in the waveform (hard to notice that in a photo :( ) because the ISR is not hitting the accumulator in the same phase. Because of that, I Will discard 1024 entries-1024 values table synthesis.

 

So I Went with 1024 values-256 entries as you proposed, and this code did it...

 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
volatile uint32_t phase_acc = 0;
volatile uint32_t phase_inc =65535*10;
volatile unsigned int reg;
//const __flash uint8_t sine[256]={127, 130, 133, 136, 140, 143, 146, 149, 152, 155, 158, 161, 164, 167, 170, 173, 176, 179, 182, 185, 187, 190, 193, 195, 198, 201, 203, 206, 208, 211, 213, 215, 218, 220, 222, 224, 226, 228, 230, 232, 233, 235, 237, 238, 240, 241, 243, 244, 245, 246, 248, 249, 249, 250, 251, 252, 253, 253, 254, 254, 254, 255, 255, 255, 255, 255, 255, 255, 254, 254, 254, 253, 253, 252, 251, 250, 249, 249, 248, 246, 245, 244, 243, 241, 240, 238, 237, 235, 233, 232, 230, 228, 226, 224, 222, 220, 218, 215, 213, 211, 208, 206, 203, 201, 198, 195, 193, 190, 187, 185, 182, 179, 176, 173, 170, 167, 164, 161, 158, 155, 152, 149, 146, 143, 140, 136, 133, 130, 128, 125, 122, 119, 115, 112, 109, 106, 103, 100, 97, 94, 91, 88, 85, 82, 79, 76, 73, 70, 68, 65, 62, 60, 57, 54, 52, 49, 47, 44, 42, 40, 37, 35, 33, 31, 29, 27, 25, 23, 22, 20, 18, 17, 15, 14, 12, 11, 10, 9, 7, 6, 6, 5, 4, 3, 2, 2, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 3, 4, 5, 6, 6, 7, 9, 10, 11, 12, 14, 15, 17, 18, 20, 22, 23, 25, 27, 29, 31, 33, 35, 37, 40, 42, 44, 47, 49, 52, 54, 57, 60, 62, 65, 68, 70, 73, 76, 79, 82, 85, 88, 91, 94, 97, 100, 103, 106, 109, 112, 115, 119, 122, 125};
const __flash uint16_t sine[256]={511, 524, 536, 549, 561, 574, 586, 599, 611, 623, 635, 648, 660, 672, 683, 695, 707, 718, 730, 741, 752, 763, 774, 785, 795, 806, 816, 826, 836, 845, 855, 864, 873, 882, 890, 899, 907, 915, 922, 930, 937, 944, 950, 956, 963, 968, 974, 979, 984, 989, 993, 997, 1001, 1004, 1008, 1011, 1013, 1015, 1017, 1019, 1021, 1022, 1022, 1023, 1023, 1023, 1022, 1022, 1021, 1019, 1017, 1015, 1013, 1011, 1008, 1004, 1001, 997, 993, 989, 984, 979, 974, 968, 963, 956, 950, 944, 937, 930, 922, 915, 907, 899, 890, 882, 873, 864, 855, 845, 836, 826, 816, 806, 795, 785, 774, 763, 752, 741, 730, 718, 707, 695, 683, 672, 660, 648, 635, 623, 611, 599, 586, 574, 561, 549, 536, 524, 512, 499, 487, 474, 462, 449, 437, 424, 412, 400, 388, 375, 363, 351, 340, 328, 316, 305, 293, 282, 271, 260, 249, 238, 228, 217, 207, 197, 187, 178, 168, 159, 150, 141, 133, 124, 116, 108, 101, 93, 86, 79, 73, 67, 60, 55, 49, 44, 39, 34, 30, 26, 22, 19, 15, 12, 10, 8, 6, 4, 2, 1, 1, 0, 0, 0, 1, 1, 2, 4, 6, 8, 10, 12, 15, 19, 22, 26, 30, 34, 39, 44, 49, 55, 60, 67, 73, 79, 86, 93, 101, 108, 116, 124, 133, 141, 150, 159, 168, 178, 187, 197, 207, 217, 228, 238, 249, 260, 271, 282, 293, 305, 316, 328, 340, 351, 363, 375, 388, 400, 412, 424, 437, 449, 462, 474, 487, 499};

void pwm_setup(void);

int main(void)
{
	OCR1A=0;
	DDRD|=0xFF;
	DDRB|=0xFF;
	TIMSK1=(1<<OCIE1A);
	OCR1A=1;

	pwm_setup();
	sei();
	while (1)

	{

		phase_acc += phase_inc;

	}
	return 0;
}

ISR(TIMER1_COMPA_vect)
{

	OCR1A= sine[phase_acc>>24];

}

void pwm_setup(void)
{
	TCCR1A |= (1 << COM1A1);
	TCCR1A |= (1 << WGM11) | (1 << WGM10);
	TCCR1B |= (1 << CS10) | (0 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

This lowered the switch frequency also, so it won't look so sharp as the 8bit one...

 

I can notice a little symmetry distortion, but the accumulator operation does not explain it. Possibly, loading the 10bit register(2 ops and the TEMP register) enlarges the ISR a bit.

Another question: How I can read buttons with this boy running in the main loop? Probably any sort of burden in the main loop will distort the waveform.
(Really, thanks for your time)

Last Edited: Tue. Jul 9, 2019 - 01:08 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why do you keep on changing it!!!!! You can't update the phase_acc in the main loop! 

 

volatile uint32_t phase_inc =65535*10;

Just to ensure you force the compiler's hand to do 32bit calcs -

volatile uint32_t phase_inc = 65535UL*10UL;

What is the clock speed of your AVR? Then we can talk about handling button inputs.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

 I can notice a little symmetry distortion, but the accumulator operation does not explain it. Possibly, loading the 10bit register(2 ops and the TEMP register) enlarges the ISR a bit.

Another question: How I can read buttons with this boy running in the main loop? Probably any sort of burden in the main loop will distort the waveform.
(Really, thanks for your time)

That's more than a little symmetry distortion, so something is not working as you hoped.

 8bit version (256 values, 256 entries)…  looks good.

With DDS, your adder usually runs at a known, INTERRUPT fixed rate, with PWM update involved here, that would sensibly be done in the PWM-Period interrupt.

There is no need to index your sine table, faster than PWM can consume the answers.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

Why do you keep on changing it!!!!! You can't update the phase_acc in the main loop! 

 

volatile uint32_t phase_inc =65535*10;

Just to ensure you force the compiler's hand to do 32bit calcs -

volatile uint32_t phase_inc = 65535UL*10UL;

What is the clock speed of your AVR? Then we can talk about handling button inputs.

 

 

 

This is running on a 20Mhz crystal, Atmega 328P. 

So the error in the first 8bit attempt was only the phase increment?
Here's : accumulator update inside ISR, 1024 values, 256 entries, high phase increment.

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
#include <math.h>
#include <util/delay.h>
#include <stdint.h>
#define F_CPU 20000000UL
volatile uint32_t phase_acc = 0;
volatile uint32_t phase_inc =65535UL*70UL;
volatile unsigned int reg;
const __flash uint16_t sine[256]={511, 524, 536, 549, 561, 574, 586, 599, 611, 623, 635, 648, 660, 672, 683, 695, 707, 718, 730, 741, 752, 763, 774, 785, 795, 806, 816, 826, 836, 845, 855, 
    864, 873, 882, 890, 899, 907, 915, 922, 930, 937, 944, 950, 956, 963, 968, 974, 979, 984, 989, 993, 997, 1001, 1004, 1008, 1011, 1013, 1015, 1017, 1019, 1021, 1022, 1022, 1023, 1023, 
    1023, 1022, 1022, 1021, 1019, 1017, 1015, 1013, 1011, 1008, 1004, 1001, 997, 993, 989, 984, 979, 974, 968, 963, 956, 950, 944, 937, 930, 922, 915, 907, 899, 890, 882, 873, 864, 855,
    845, 836, 826, 816, 806, 795, 785, 774, 763, 752, 741, 730, 718, 707, 695, 683, 672, 660, 648, 635, 623, 611, 599, 586, 574, 561, 549, 536, 524, 512, 499, 487, 474, 462, 449, 437, 
    424, 412, 400, 388, 375, 363, 351, 340, 328, 316, 305, 293, 282, 271, 260, 249, 238, 228, 217, 207, 197, 187, 178, 168, 159, 150, 141, 133, 124, 116, 108, 101, 93, 86, 79, 73, 67, 
    60, 55, 49, 44, 39, 34, 30, 26, 22, 19, 15, 12, 10, 8, 6, 4, 2, 1, 1, 0, 0, 0, 1, 1, 2, 4, 6, 8, 10, 12, 15, 19, 22, 26, 30, 34, 39, 44, 49, 55, 60, 67, 73, 79, 86, 93, 101, 108, 
    116, 124, 133, 141, 150, 159, 168, 178, 187, 197, 207, 217, 228, 238, 249, 260, 271, 282, 293, 305, 316, 328, 340, 351, 363, 375, 388, 400, 412, 424, 437, 449, 462, 474, 487, 499};

void pwm_setup(void);

int main(void)
{
    OCR1A=0;
    DDRD|=0xFF;
    DDRB|=0xFF;
    TIMSK1=(1<<OCIE1A);
    OCR1A=1;
    pwm_setup();
    sei();
    while (1)
    
    {

        //empty
        
    }
    return 0;
}


ISR(TIMER1_COMPA_vect)
{

    phase_acc += phase_inc;
    OCR1A= sine[phase_acc>>24];
    
}

void pwm_setup(void)
{
    TCCR1A |= (1 << COM1A1);
    TCCR1A |= (1 << WGM11) | (1 << WGM10);
    TCCR1B |= (1 << CS10) | (0 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}


  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Who-me wrote:
With DDS, your adder usually runs at a known, INTERRUPT fixed rate, with PWM update involved here, that would sensibly be done in the PWM-Period interrupt.
More than sensible.  If PWM runs at anything but an integer multiple of the adder interrupt, (additional) harmonic noise will be injected into the output.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

measure how long the isr takes to execute. As mentioned before, you can toggle a port bit before and after the code in the isr but this doesn't take into account the isr overhead which can be substantial. Other ways are to count the cycles using the compiler generated asm file, or using the simulator (probably the easiest) in Studio 7. With a 20MHz clock and depending on the actual pwm frequency will determine what percentage of cycles the DDS uses. 

 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
#include <math.h>
#include <util/delay.h>
#include <stdint.h>
#define F_CPU 20000000UL

volatile uint32_t phase_inc =65535UL*70UL;
volatile uint32_t counter = 0;
volatile unsigned int reg;
const __flash uint16_t sine[256]={511, 524, 536, 549, 561, 574, 586, 599, 611, 623, 635, 648, 660, 672, 683, 695, 707, 718, 730, 741, 752, 763, 774, 785, 795, 806, 816, 826, 836, 845, 855, 
    864, 873, 882, 890, 899, 907, 915, 922, 930, 937, 944, 950, 956, 963, 968, 974, 979, 984, 989, 993, 997, 1001, 1004, 1008, 1011, 1013, 1015, 1017, 1019, 1021, 1022, 1022, 1023, 1023, 
    1023, 1022, 1022, 1021, 1019, 1017, 1015, 1013, 1011, 1008, 1004, 1001, 997, 993, 989, 984, 979, 974, 968, 963, 956, 950, 944, 937, 930, 922, 915, 907, 899, 890, 882, 873, 864, 855,
    845, 836, 826, 816, 806, 795, 785, 774, 763, 752, 741, 730, 718, 707, 695, 683, 672, 660, 648, 635, 623, 611, 599, 586, 574, 561, 549, 536, 524, 512, 499, 487, 474, 462, 449, 437, 
    424, 412, 400, 388, 375, 363, 351, 340, 328, 316, 305, 293, 282, 271, 260, 249, 238, 228, 217, 207, 197, 187, 178, 168, 159, 150, 141, 133, 124, 116, 108, 101, 93, 86, 79, 73, 67, 
    60, 55, 49, 44, 39, 34, 30, 26, 22, 19, 15, 12, 10, 8, 6, 4, 2, 1, 1, 0, 0, 0, 1, 1, 2, 4, 6, 8, 10, 12, 15, 19, 22, 26, 30, 34, 39, 44, 49, 55, 60, 67, 73, 79, 86, 93, 101, 108, 
    116, 124, 133, 141, 150, 159, 168, 178, 187, 197, 207, 217, 228, 238, 249, 260, 271, 282, 293, 305, 316, 328, 340, 351, 363, 375, 388, 400, 412, 424, 437, 449, 462, 474, 487, 499};

void pwm_setup(void);

int main(void)
{
    OCR1A=0;
    DDRD|=0xFF;
    DDRB|=0xFF;
    TIMSK1=(1<<OCIE1A);
    OCR1A=1;
    pwm_setup();
    sei();
    while (1)
    
    {

        //empty
        
    }
    return 0;
}


ISR(TIMER1_COMPA_vect)
{
    static uint32_t phase_acc = 0; //it makes sense that this lives here as it is not used outside of the isr

    phase_acc += phase_inc;
    OCR1A= sine[phase_acc>>24];
    
    counter++;      //to keep track of time
    
}

void pwm_setup(void)
{
    TCCR1A |= (1 << COM1A1);
    TCCR1A |= (1 << WGM11) | (1 << WGM10);
    TCCR1B |= (1 << CS10) | (0 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

uint32_t get_tick_count(void)
{
    uint32_t ticks;
    
    cli();
    ticks = counter;    //ensure we read counter atomically
    sei();
    return ticks;
}
    

you can then use get_tick_count() much like millis() on the Arduino.  So in Main() you can read the current tick count ,save it to a variable. Then in a loop, you can read the current tick count and use the save count to calculate elapsed time.

 

To read pushbuttons, you might want to read them every 10ms or so and apply a debounce algorithm to it. Debouncing consists of reading the inputs a number of times over a time period and keep count of how many times in a row you get a '1' or a '0'. Once you've got a nice debounced value from your pushbuttons, you can then increment/decrement etc what you want.

 

You can apply a technique like this:

https://www.avrfreaks.net/forum/...

 

rather than using a specific timer to generate the tick, you use the get_tick_count() to derive the 10ms tick time.

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You have another problem.  You are using data in both main code and ISR code, and you are not protecting the shared data from corruption.

 

int main(void)
{
....
    while (1)
    {
        phase_acc += phase_inc;
    }
....
}

ISR(TIMER1_COMPA_vect)
{
....
    reg = sine[phase_acc>>24];
....
}

 In your case, the problem can (no, WILL) happen as your main code is writing out the result of phase_acc += phase_inc; If (no, WHEN) the interrupt happens while the main code is in the middle of writing out the multi-byte phase_acc, the ISR will read an incorrect value, half old value and half new value.  The solution is to prevent interrupts (really, only the interrupt that accesses the data in question) while main code is modifying the data.  The simplest form is as follows:

 

int main(void)
{
....
    while (1)
    {
        cli();          // don't let ISR access corrupt data
        phase_acc += phase_inc;
        sei();
    }
....
}

ISR(TIMER1_COMPA_vect)
{
....
    reg = sine[phase_acc>>24];
....
}

 

Last Edited: Tue. Jul 9, 2019 - 05:09 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Weird stuff:

The distortion source at low frequencies(below 100Hz), was the LC filter+buffer itself. Apparently is phase distortion at different pulse densities. The unbuffered version has better form, even at low frequencies:

 

I measured the ISR overhead (1024 values, 256 entries case), with gain division included(integer), it takes 2.200us to process the complete ISR, without taking account of CLI and SEI. Something strange occurs: ISR is called twice even if interruptions are disabled inside. I have noticed this debugging with my Atmel ICE, but I thought was only a sort of debugWire glitch. have I to reset some interrupt flag(another tan SEI) manually in the ISR (like x86 interrupt controllers)?

 


 

Last Edited: Tue. Jul 9, 2019 - 09:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The distortion source at low frequencies(below 100Hz), was the LC filter+buffer itself.

Post your circuit...the issue may be lurking

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What do you mean by ‘interrupts disabled inside”?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

 

The upper is the current version. The lower is the previous distorting version.

 

Kartman wrote:
What do you mean by ‘interrupts disabled inside”?


ISR(TIMER1_COMPA_vect)
{
    cli();
    //PORTD=~PIND;
    static uint32_t phase_acc = 0;
    static uint32_t phase_inc =65535UL*1UL;
    static uint8_t gain=1;
    phase_acc += phase_inc;
    OCR1A= sine[phase_acc>>24]/gain;
    //counter++;
    //PORTD=~PIND;
    sei();
}

 

 

 

Last Edited: Tue. Jul 9, 2019 - 10:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

The upper is the current version. The lower is the previous distorting version.

 

Maybe that 74LS04 does not like the inductive load ? - whilst the CMOS port structure of the AVR is more tolerant.  Better to use a chained RC filter than a LC one, as RC present a 'nicer' load .....

Even with a resistive load, I'd avoid the ancient 74LS04 as a DAC driver, as the output load line of that is messy at best...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

ISR(TIMER1_COMPA_vect)
{
    cli();
    //PORTD=~PIND;
    static uint32_t phase_acc = 0;
    static uint32_t phase_inc =65535UL*1UL;
    static uint8_t gain=1;
    phase_acc += phase_inc;
    OCR1A= sine[phase_acc>>24]/gain;
    //counter++;
    //PORTD=~PIND;
    sei();
}

GAH!  NO!

 

AVR Libc's interrupt machinery provided by, among other bits, the 'ISR()' paradigm already handles disabling and re-enabling of interrupts.  In fact, interrupts are disabled by hardware before vectoring to the interrupt handler.  They are re-enabled by the RETI tacked onto the end of the handler (by AVR Libc).

 

What you're doing is re-enabling interrupts while still inside an interrupt handler.  This is very bad juju.

 

Drop the cli() and sei() from your ISR.  From >>all<< your ISRs.

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Tue. Jul 9, 2019 - 11:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

Your 25ms time scale way way way off to see what is going on...are these pulses 10us?  50ns? You will never tell on that scale, use maybe 100ns/div or maybe 10us/div as needed

 

You don't show how the chip is powered---what caps are you using for that?   How is your scope connected...high speed edges will easily fool you, depending how & where the scope is grounded.

 

Also, how you power up  & connect the AVR matters!  Where are your bypass caps?  You did not show a schematic!!!

 

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Tue. Jul 9, 2019 - 11:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

re-enabling interrupts while still inside an interrupt handler.  This is very bad juju.

Oh it is so much fun for the unwary, let the stack games begin!  Winner uses all. 

 

Even with a resistive load, I'd avoid the ancient 74LS04 as a DAC driver

Nobody would use that to generate an accurate PWM voltage, since we don't even know what voltage the chip outputs (it puts out a wide range)... CMOS sticks much closer to the rails (gnd & Vcc), if lightly loaded. 

 

In many cases knowing an accurate voltage is not important, in a feedback loop it just increases/decreases until the SMD oven is at the right temperature & your pizza gets cooked.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Tue. Jul 9, 2019 - 11:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, I did warn about the 74LS04..... Use a 74HC04 or similar.

At first glance, the values of your filter seem a bit nasty. I'm at the office, so I don't have the time to do some calcs. I would've just used a simple RC filter for testing. Wait till you get to the power driver - mistakes here are more entertaining! I remember blowing up a number of 200A transistors when playing with a 3kW inverter. The transistors were $100 a pop.

 

 

here's something I wrote earlier:

 

https://www.avrfreaks.net/forum/...

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

ISR(TIMER1_COMPA_vect)
{
    cli();
    //PORTD=~PIND;
    static uint32_t phase_acc = 0;
    static uint32_t phase_inc =65535UL*1UL;
    static uint8_t gain=1;
    phase_acc += phase_inc;
    OCR1A= sine[phase_acc>>24]/gain;
    //counter++;
    //PORTD=~PIND;
    sei();
}

 

Ah, you've pulled a fast one on us.  The code I commented on shared phase_acc between main code and ISR code, resulting in danger of corruption and requiring (usually) explicit protection against an interrupt while main code is accessing the data.  Now you've moved the main code inside the ISR, so there is no danger of corruption, and the cli() and sei() which were necessary in main code are not only unnecessary in the ISR, but are a recipe for disaster and need to be removed.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also, why 65535?  I would think you'd want 65536.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 static uint32_t phase_inc =65535UL*1UL;
    static uint8_t gain=1;

 

Why would you do this? Surely you want to vary the gain and the frequency? If so, then don't make them local to the isr. I give you the magic sauce, but then you figure that more is better. Use the right magic sauce in the right place at the right time.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

Well, I did warn about the 74LS04..... Use a 74HC04 or similar.


The 74LS04 is quite slow and the inductor did it even worse. I tested inductor alone and, despite of it not alter symmetry, it add some nonlinearities that made sine to have some rounding at peaks.

 

joeymorin wrote:

ColdfireMC wrote:

ISR(TIMER1_COMPA_vect)
{
    cli();
    //PORTD=~PIND;
    static uint32_t phase_acc = 0;
    static uint32_t phase_inc =65535UL*1UL;
    static uint8_t gain=1;
    phase_acc += phase_inc;
    OCR1A= sine[phase_acc>>24]/gain;
    //counter++;
    //PORTD=~PIND;
    sei();
}

GAH!  NO!

 

AVR Libc's interrupt machinery provided by, among other bits, the 'ISR()' paradigm already handles disabling and re-enabling of interrupts.  In fact, interrupts are disabled by hardware before vectoring to the interrupt handler.  They are re-enabled by the RETI tacked onto the end of the handler (by AVR Libc).

 

What you're doing is re-enabling interrupts while still inside an interrupt handler.  This is very bad juju.

 

Drop the cli() and sei() from your ISR.  From >>all<< your ISRs.

 

 

Talking about interrupts, just forgot that. I misunderstood the "protect the ISR variables" advice, thinking that I forgot to put ISR prologue and epilogue, when actually was a variable scope issue. 

 

avrcandies wrote:

 

Your 25ms time scale way way way off to see what is going on...are these pulses 10us?  50ns? You will never tell on that scale, use maybe 100ns/div or maybe 10us/div as needed

 

You don't show how the chip is powered---what caps are you using for that?   How is your scope connected...high speed edges will easily fool you, depending how & where the scope is grounded.

 

Also, how you power up  & connect the AVR matters!  Where are your bypass caps?  You did not show a schematic!!!

 

 

 

I'm using an Atmega 328P connected to an STK500, but programming is being done with an Atmel ICE. The filter is mounted on a breadboard. The buffer was mounted without decoupling cap, but actually the buffer itself was the problem, so I retired it. Scale is not "comfortable", because pulses were around 10khz there, but the sine filtered waveform (down) is running at 14/15 Hz.I lowered the switching frequency using the prescaler, now I'm running at 1.2Khz switching.

I don't have my 465M here. A double base Scope like that can show this stuff more comfortably.

 

 

Kartman wrote:

At first glance, the values of your filter seem a bit nasty. I'm at the office, so I don't have the time to do some calcs. I would've just used a simple RC filter for testing. Wait till you get to the power driver - mistakes here are more entertaining! I remember blowing up a number of 200A transistors when playing with a 3kW inverter. The transistors were $100 a pop.

here's something I wrote earlier:

https://www.avrfreaks.net/forum/...

I'm using an RC filter now and sine looks much better, 7Hz cutoff.

That's the why I ask. I don't want to kill anyone yet devil.
 

Last Edited: Wed. Jul 10, 2019 - 09:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Get it to sweep the frequency. Then marvel in the magic of dds!

The waveform gets more ragged the higher in frequency you go.

Last Edited: Thu. Jul 11, 2019 - 02:17 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This is not a filter fault ! 

If a filter makes it better it's because it blur the sw error.

Find the error first. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm experiencing a little jitter with higher frequencies. I have a 1.2khz pwm frequency using internal prescaler (/8 prescaler). Clock is a 20Mhz crystal mounted in the crystal socket of STK500. from 50/60hz onwards, the resulting sine starts to get some jitter

 

Setup photo

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Your filter values are too low - this is loading the output too much. You can see this in the scope trace of the pwm.

Nevertheless, with dds, as the ratio of the output freq gets closer to the sample freq the ‘quality’ of the waveform gets worse. This is because you have larger jumps through your sine table. Eg: 1200Hz sample, 50Hz output is a ratio of 24:1. This means you skip, say, every 10 samples in the sinetable. Depending on the integer roundup of the dds calc, cycle to cycle you use different samples. By tweeking the phase-inc value slightly, you can see the effect this has on the ‘glitching’. This is an artifact of a sampled time system. You should be able to simulate all of this on your PC using matlab or the likes thereof.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'd be curious to see the code that's producing the waveform in #54.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

Changed filter values, now output stage is less loaded. However waves still are a little "jumpy" at higher frequencies. I tried to "fix" this lowering crystal to 10Mhz and getting back prescaler to 1. That gives 4Khz PWM, but the problem persists...

I don't remember to have this problem with 8 bit PWM. I will give it a try.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In the isr, copy bit 31 of the phase_acc to a port pin. Trigger off that. See if you still get jitter.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
In the isr, copy bit 31 of the phase_acc to a port pin. Trigger off that. See if you still get jitter.

If this code does that

ISR(TIMER1_COMPA_vect)
{
  	static uint32_t phase_acc = 0;
	phase_acc += phase_inc;
	OCR1A= sine[phase_acc>>24]/gain;
	PORTD=(phase_acc>>24);
	//counter++;
}

(with probe connected at PORTD7) Jitter is less, but still present.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How much jitter? Engineers talk in numbers. Is the jitter in the output waveform or in the PORTD.7 signal?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

is this number difference between pulse widths? Now I'm with 20Mhz back, 10bit normal PWM, and /8 prescaler.

I have PORTD.7 pulses of 3.2ms, 3.60ms, 3.7ms, so 0.5ms of jitter.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You're going to have to do some detective work. 500us is how many pwm periods? It is unlikely the pwm hardware is missing ticks, so that might suggest that you're missing interrupts.  Toggle a port pin in your isr and see if you're missing interrupts.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

ISR(TIMER1_COMPA_vect)
{
  	static uint32_t phase_acc = 0;
	phase_acc += phase_inc;
	OCR1A= sine[phase_acc>>24]/gain;
	PORTD=(phase_acc>>24);
	//counter++;
}

 

The jitter is perhaps caused by the ISR triggering

on COMPA which is not constant.  You want to use

a regular interval such as overflow (OVF) or COMPB

(set to a single value) for this routine.

 

Also, calculate the next value after updating the

current value like this:

 

ISR(TIMER1_COMPB_vect) {
    static uint32_t phase_acc = 0;
    static uint16_t next_output = 0;

    OCR1A = next_output; // jitter-B-gone!

    phase_acc += phase_inc;
    next_output = sine[phase_acc>>24]/gain;
}

 

This introduces a delay in your output by one sample

period.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Nice catch Mike! I didn't spot that one. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

done. Jitter went down, however is still present (480us). I have some doubts about what mode of pwm is the correct.

 

void pwm_setup(void)
{
        TIMSK1=(1<<OCIE1A);
	TCCR1A |= (1 << COM1A1);
	TCCR1A |= (1 << WGM11) | (1 << WGM10);
	TCCR1B |= (0 << CS10) | (1 << CS11) | (0 << CS12) | (0 << WGM13) | (1 << WGM12);
}

This supposedly is PWM 10 bit phase correct, /8 prescaler, output compare mode (mode 7). Now i'm generating the interrupt using the output compare. Is this the recommended way to do it, or there's some other way to do it?
 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You’re still using compare A for the interrupt source. Use the ovf (overflow) interrupt. The reason is the interrupt is generated at a consistent point in the pwm cycle. When you were using compa this would interrupt at a variable point in the pwm cycle dependent on the actual pwm value. The other (potential) issue is the update of the pwm value. By the time the interrupt fires and you enter the isr, your code executes and updates the ocr1a value. Depending on the window of opportunity, you might miss updating the pwm on the next cycle. So the point hat Mike makes is to update the ocr1a value early in the isr. The other point Mike makes is you can use ocr1b to fire an interrupt at a point in the pwm cycle to ensure you update ocr1a at the required time.

Now - don’t do this all at once as you’ll tie yourself up in knots if it doesn’t work as planned. First just change your code to use timer1_ovf. Change the isr vector name and the timer interrupt enable bit.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Mode 7 is 10-bit Fast PWM.  OCR1x is buffered so

changes happen at the right moment (BOTTOM).

 

For a nice smooth output waveform, you need to

update OCR1A exactly once per timer period. It

looks like you're still using the COMPA interrupt

to make changes to the OCR1A value.  This isn't

a good idea because if OCR1A is near to the TOP

value, your interrupt might not update OCR1A to

the next output value quickly enough.  Better to

use the overflow or COMPB interrupt (with a low

enough value) to ensure OCR1A is updated before

the timer wraps around.

 

--Mike

 

EDIT: Too slow!

 

Last Edited: Fri. Jul 12, 2019 - 09:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And I’d just woken up!!!!!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ISR(TIMER1_OVF_vect)
{
	static uint16_t next_reg = 0;
	static uint32_t phase_acc = 0;

	OCR1A=next_reg;
	phase_acc += phase_inc;
	next_reg=sine[phase_acc>>24]/gain;
	PORTD=(phase_acc>>24);
	//counter++;
}

ISR changed. At some frequencies almost dissapear, but from 60hz onwards is present. I'm triggering using the 31st bit as trigger.

 

void pwm_setup(void)
{
	//TIMSK1=(1<<OCIE1A);
	TIMSK1=(1<<TOIE1);
	TCCR1A |= (1 << COM1A1);
	TCCR1A |= (1 << WGM11) | (1 << WGM10);
	TCCR1B |= (0 << CS10) | ( 1 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

 

 

This is PORTB7. The narrowest pulse is 480us, the widest is 580us. 93Hz

 

Last Edited: Sat. Jul 13, 2019 - 12:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Take out the gain division for the moment. On an AVR divides are expensive, so for gain you would multiply as the AVR has multiply hardware then divide by shifting right a multiple of 8 bits.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EDIT: I was wrong in measurements

Well, I not have the 210 here, but the jitter is visible. Every 2 seconds, waveform jitters 0.8ms, something like that. Retired division from code. No code is executing in main loop. 93 hz again. With a 16bit phase accumulator Jitter can be less?

Last Edited: Sat. Jul 13, 2019 - 09:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

if you have 93Hz, then your scope scale that your showing must be 2ms/division

You probably can't really see 50us jitter on a 2ms scale...that would only be 1/40 of one square!

 

What are you triggering on?  If the wave is jagged, each sweep it can trigger at a slightly different point 

 

 Every 2 seconds, waveform jitters 50us  

 How would you know that using the scope as shown?

 

I'm not saying you don't have jitter, i'm saying it can be hard to measure & you can fool yourself.   If you have a known solid reference (to trigger recording each repetition) that makes it easier, but that assumes the reference is jitter-free

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ColdfireMC wrote:

Every 2 seconds, waveform jitters 50us

 

Probably due to dropping the lower 24 bits of phase_acc.

 

Ideally you'd use those bits to interpolate the sample

output between the surrounding two values.  Linear

interpolation is straightforward to implement, but

does require some time to calculate.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

or tweek the phase increment.

 

If the end use for this is to drive an induction motor, then 50us is trivial.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Have I lost the plot?
The OP just wants to create a 50Hz sine wave using PWM and low pass filter.
Surely this is one lookup table with about 20 lines of code for the software.
And one RC for the low pass filter hardware.
How do you get 74 messages?
.
David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
How do you get 74 messages?
.

 

Some need a little more help than others......

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

please show the code you use (inclusive, sine-table and how you make the trigger signal).(any other ISR's enabled?, empty main ? how "empty" :) )

 

How long does the ISR take, set a port pin as first and last thing in the ISR (which optimizer level do you use ? -O2 )

If speed really matter then place the two static variables in registers that should at least speed it up with a factor of 2 (in ASM the hole ISR would take about 20 clk so 1us) 

 

I fear that you still have some sw errors, so my comment in #53 still count, I fear that you just blur a bad signal with a filter.

 

I have not read the datasheet for this setup, but are you sure that when you do :

OCR1A=next_reg;

that the timer haven't passed the switch value ? (if it's buffered it's not at problem!).

 

 

a note to David, I think OP have at lot to learn, at I hope he will learn some from this thread.

If it's just was 50Hz sinewave it would take me less than an hour to code (I would do this kind of stuff in ASM, HW close things are easier that way), but it could easy take hours to understand the settings of PWM and timers etc. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I second the call to see the full software listing.  Right now we are all just looking at different parts of the elephant.  It's not even clear to me that the jitter being reported is real, as opposed to an artifact of the triggering scheme.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
#include <math.h>
#include <util/delay.h>
#include <stdint.h>
#define F_CPU 20000000UL
#define DEFAULT_STEP 1
#define DEFAULT_INC 65535UL*10000UL
volatile uint32_t phase_inc=DEFAULT_INC;
volatile uint8_t gain=1;
void pwm_setup(void);
void timer_setup(void);
uint32_t get_tick_count(void);

const __flash uint16_t sine[256]={511, 524, 536, 549, 561, 574, 586, 599, 611, 623, 635, 648, 660, 672, 683, 695, 707, 718, 730, 741, 752, 763, 774, 785, 795, 806, 816, 826, 836, 845, 855,
    864, 873, 882, 890, 899, 907, 915, 922, 930, 937, 944, 950, 956, 963, 968, 974, 979, 984, 989, 993, 997, 1001, 1004, 1008, 1011, 1013, 1015, 1017, 1019, 1021, 1022, 1022, 1023, 1023,
    1023, 1022, 1022, 1021, 1019, 1017, 1015, 1013, 1011, 1008, 1004, 1001, 997, 993, 989, 984, 979, 974, 968, 963, 956, 950, 944, 937, 930, 922, 915, 907, 899, 890, 882, 873, 864, 855,
    845, 836, 826, 816, 806, 795, 785, 774, 763, 752, 741, 730, 718, 707, 695, 683, 672, 660, 648, 635, 623, 611, 599, 586, 574, 561, 549, 536, 524, 512, 499, 487, 474, 462, 449, 437,
    424, 412, 400, 388, 375, 363, 351, 340, 328, 316, 305, 293, 282, 271, 260, 249, 238, 228, 217, 207, 197, 187, 178, 168, 159, 150, 141, 133, 124, 116, 108, 101, 93, 86, 79, 73, 67,
    60, 55, 49, 44, 39, 34, 30, 26, 22, 19, 15, 12, 10, 8, 6, 4, 2, 1, 1, 0, 0, 0, 1, 1, 2, 4, 6, 8, 10, 12, 15, 19, 22, 26, 30, 34, 39, 44, 49, 55, 60, 67, 73, 79, 86, 93, 101, 108,
    116, 124, 133, 141, 150, 159, 168, 178, 187, 197, 207, 217, 228, 238, 249, 260, 271, 282, 293, 305, 316, 328, 340, 351, 363, 375, 388, 400, 412, 424, 437, 449, 462, 474, 487, 499};
/*
const uint8_t sine[256]={127, 130, 133, 136, 140, 143, 146, 149, 152, 155, 158, 161, 164, 167, 170, 173, 176, 179, 182, 185, 187, 190, 193, 195, 198, 201, 203, 206, 208, 211, 213, 215, 218, 220, 222, 224, 226, 228, 230, 232, 233, 235, 237, 238, 240, 241, 243, 244, 245, 246, 248, 249, 249, 250, 251, 252, 253, 253, 254, 254, 254, 255, 255, 255, 255, 255, 255, 255, 254, 254, 254, 253, 253, 252, 251, 250, 249, 249, 248, 246, 245, 244, 243, 241, 240, 238, 237, 235, 233, 232, 230, 228, 226, 224, 222, 220, 218, 215, 213, 211, 208, 206, 203, 201, 198, 195, 193, 190, 187, 185, 182, 179, 176, 173, 170, 167, 164, 161, 158, 155, 152, 149, 146, 143, 140, 136, 133, 130, 128, 125, 122, 119, 115, 112, 109, 106, 103, 100, 97, 94, 91, 88, 85, 82, 79, 76, 73, 70, 68, 65, 62, 60, 57, 54, 52, 49, 47, 44, 42, 40, 37, 35, 33, 31, 29, 27, 25, 23, 22, 20, 18, 17, 15, 14, 12, 11, 10, 9, 7, 6, 6, 5, 4, 3, 2, 2, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 3, 4, 5, 6, 6, 7, 9, 10, 11, 12, 14, 15, 17, 18, 20, 22, 23, 25, 27, 29, 31, 33, 35, 37, 40, 42, 44, 47, 49, 52, 54, 57, 60, 62, 65, 68, 70, 73, 76, 79, 82, 85, 88, 91, 94, 97, 100, 103, 106, 109, 112, 115, 119, 122, 125};
*/
int main(void)
{
    OCR1A=0;
    //DDRC|=0xFF;
    DDRD|=0xFF;
    DDRB|=0xFF;
    
    OCR1A=1;
    pwm_setup();
    timer_setup();
    PORTB=0;
    sei();
    PORTD=0;
    while (1)
    
    {
        
        
        //if (poll_key==1)
        //{
        //checkbutton();
        //}
        
        //empty
        //counter_main=get_tick_count(); (4294967296-1)/4)
    }
    return 0;
}

ISR(TIMER1_OVF_vect)
{
    static uint16_t next_reg = 0;
    static uint32_t phase_acc = 0;
    
    OCR1A=next_reg;
    phase_acc += phase_inc;
    next_reg=sine[phase_acc>>24];
    //PORTD=(phase_acc>>24);
    //PORTD=next_reg;
    //counter++;
}

void timer_setup(void)
{
    //TIMSK0=(1<<OCIE0A);
    TCCR0A |= (1 << COM1A1);
    TCCR0A |= (1 << WGM11) | (0 << WGM00) | (1<<COM0A0) | (0<<COM1A0);
    TCCR0B |= (0 << CS10) | (0 << CS11) | (1 << CS12) | (0 << WGM02);
    OCR0A=16;
    //    OCR0B=64;
}
void pwm_setup(void)
{
    //TIMSK1=(1<<OCIE1A);
    TIMSK1=(1<<TOIE1);
    TCCR1A |= (1 << COM1A1);
    TCCR1A |= (1 << WGM11) | (1 << WGM10);
    TCCR1B |= (0 << CS10) | ( 1 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

The current code. Supposedly, TIMER0 is initialized, but no interrupt is enabled. Now I'm using O3, release option.

 

Kartman wrote:

or tweek the phase increment.

 

If the end use for this is to drive an induction motor, then 50us is trivial.

That was the question.

 

avrcandies wrote:

if you have 93Hz, then your scope scale that your showing must be 2ms/division

You probably can't really see 50us jitter on a 2ms scale...that would only be 1/40 of one square!

 

What are you triggering on?  If the wave is jagged, each sweep it can trigger at a slightly different point 

 

 Every 2 seconds, waveform jitters 50us  

 How would you know that using the scope as shown?

 

I'm not saying you don't have jitter, i'm saying it can be hard to measure & you can fool yourself.   If you have a known solid reference (to trigger recording each repetition) that makes it easier, but that assumes the reference is jitter-free

 

I was wrong, corrected.

Last Edited: Sat. Jul 13, 2019 - 09:57 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Have I lost the plot? The OP just wants to create a 50Hz sine wave using PWM and low pass filter. Surely this is one lookup table with about 20 lines of code for the software. And one RC for the low pass filter hardware. How do you get 74 messages? . David.

Sine appears now. I knew that the code would be short, but has to be the correct code.

Last Edited: Sat. Jul 13, 2019 - 10:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I was wrong, corrected.

This seems a bit obtuse--how can we know what you mean...wrong about what?  What was corrected? Do you mean you no longer see jitter?  

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:

I was wrong, corrected.

This seems a bit obtuse--how can we know what you mean...wrong about what?  What was corrected? Do you mean you no longer see jitter?  

I edited that post. I was worng aobutr 50us. Added the "I think" correct measurement. The bit 31 of the Phase accumulator jitters. I'm using that as external trigger and sine wave.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I edited that post. I was worng aobutr 50us. Added the "I think" correct measurement. The bit 31 of the Phase accumulator jitters. I'm using that as external trigger and sine wave.

Ok, now why do you think you see jitter?  What are you triggering off of as a reference?  Can you capture several cycles that clearly shows it?  If you capture 4 cycles at a time , you may get lucky (after a few tries) to capture one that is different and be able to see what is going on.

 

If you listen to your sinewave, do you hear anything going wrong? 

 

I wonder if you get a rounding "residual"  when you shift by 24 bits...maybe round up 

 

You say you have 93Hz with 0.8ms jitter??...that's 26.8 degrees variation?

 

Before you even try making a sine, ensure your IRQ is rock solid...each IRQ toggle a pin & check the toggling.  You can squeeze a bunch of toggles on the scope screen & see if any look out of place.

 

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Sun. Jul 14, 2019 - 01:50 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Change 65535 to 65536 to remove some jitter.

 

Also, the update of OCR1A is buffered, but the

write to PORTD is not, so they are out of sync.

 

Not sure you can do any better, though maybe

write the "old" PORTD value just after setting

OCR1A (before incrementing phase_acc).

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have put a speaker, without filter, just with decoupling cap. Obviously tons of harmonic noise, over the fundamental, but I thin that it hears stable.

 

This is using no prescaler. Switch frequency here is high so I will not use it with those settings

 

https://1drv.ms/u/s!AjVt5WQyiI6mgq5zY4fSlDmvYvB3pQ

 

 

Here's the current code (93 Hz, prescaler /8), buffered PORTD)

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
#include <math.h>
#include <util/delay.h>
#include <stdint.h>
#define F_CPU 20000000UL
#define DEFAULT_STEP 1
#define DEFAULT_INC 65536UL*5000UL
volatile uint32_t phase_inc=DEFAULT_INC;
volatile uint8_t gain=1;
void pwm_setup(void);
void timer_setup(void);
uint32_t get_tick_count(void);

const __flash uint16_t sine[256]={511, 524, 536, 549, 561, 574, 586, 599, 611, 623, 635, 648, 660, 672, 683, 695, 707, 718, 730, 741, 752, 763, 774, 785, 795, 806, 816, 826, 836, 845, 855,
	864, 873, 882, 890, 899, 907, 915, 922, 930, 937, 944, 950, 956, 963, 968, 974, 979, 984, 989, 993, 997, 1001, 1004, 1008, 1011, 1013, 1015, 1017, 1019, 1021, 1022, 1022, 1023, 1023,
	1023, 1022, 1022, 1021, 1019, 1017, 1015, 1013, 1011, 1008, 1004, 1001, 997, 993, 989, 984, 979, 974, 968, 963, 956, 950, 944, 937, 930, 922, 915, 907, 899, 890, 882, 873, 864, 855,
	845, 836, 826, 816, 806, 795, 785, 774, 763, 752, 741, 730, 718, 707, 695, 683, 672, 660, 648, 635, 623, 611, 599, 586, 574, 561, 549, 536, 524, 512, 499, 487, 474, 462, 449, 437,
	424, 412, 400, 388, 375, 363, 351, 340, 328, 316, 305, 293, 282, 271, 260, 249, 238, 228, 217, 207, 197, 187, 178, 168, 159, 150, 141, 133, 124, 116, 108, 101, 93, 86, 79, 73, 67,
	60, 55, 49, 44, 39, 34, 30, 26, 22, 19, 15, 12, 10, 8, 6, 4, 2, 1, 1, 0, 0, 0, 1, 1, 2, 4, 6, 8, 10, 12, 15, 19, 22, 26, 30, 34, 39, 44, 49, 55, 60, 67, 73, 79, 86, 93, 101, 108,
    116, 124, 133, 141, 150, 159, 168, 178, 187, 197, 207, 217, 228, 238, 249, 260, 271, 282, 293, 305, 316, 328, 340, 351, 363, 375, 388, 400, 412, 424, 437, 449, 462, 474, 487, 499};
/*
const uint8_t sine[256]={127, 130, 133, 136, 140, 143, 146, 149, 152, 155, 158, 161, 164, 167, 170, 173, 176, 179, 182, 185, 187, 190, 193, 195, 198, 201, 203, 206, 208, 211, 213, 215, 218, 220, 222, 224, 226, 228, 230, 232, 233, 235, 237, 238, 240, 241, 243, 244, 245, 246, 248, 249, 249, 250, 251, 252, 253, 253, 254, 254, 254, 255, 255, 255, 255, 255, 255, 255, 254, 254, 254, 253, 253, 252, 251, 250, 249, 249, 248, 246, 245, 244, 243, 241, 240, 238, 237, 235, 233, 232, 230, 228, 226, 224, 222, 220, 218, 215, 213, 211, 208, 206, 203, 201, 198, 195, 193, 190, 187, 185, 182, 179, 176, 173, 170, 167, 164, 161, 158, 155, 152, 149, 146, 143, 140, 136, 133, 130, 128, 125, 122, 119, 115, 112, 109, 106, 103, 100, 97, 94, 91, 88, 85, 82, 79, 76, 73, 70, 68, 65, 62, 60, 57, 54, 52, 49, 47, 44, 42, 40, 37, 35, 33, 31, 29, 27, 25, 23, 22, 20, 18, 17, 15, 14, 12, 11, 10, 9, 7, 6, 6, 5, 4, 3, 2, 2, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 3, 4, 5, 6, 6, 7, 9, 10, 11, 12, 14, 15, 17, 18, 20, 22, 23, 25, 27, 29, 31, 33, 35, 37, 40, 42, 44, 47, 49, 52, 54, 57, 60, 62, 65, 68, 70, 73, 76, 79, 82, 85, 88, 91, 94, 97, 100, 103, 106, 109, 112, 115, 119, 122, 125};
*/
int main(void)
{
	OCR1A=0;
	//DDRC|=0xFF;
	DDRD|=0xFF;
	DDRB|=0xFF;
	
	OCR1A=1;
	pwm_setup();
	timer_setup();
	PORTB=0;
	sei();
	PORTD=0;
	while (1)
	
	{
		
		
		//if (poll_key==1)
		//{
		//checkbutton();
		//}
		
		//empty
		//counter_main=get_tick_count(); (4294967296-1)/4)
	}
	return 0;
}


ISR(TIMER1_OVF_vect)
{
	static uint16_t next_reg = 0;
	static uint32_t phase_acc = 0;
	static uint8_t next_port_out = 0;
	
	OCR1A=next_reg;
	PORTD=next_port_out;
	phase_acc += phase_inc;
	next_reg=sine[phase_acc>>24];
	next_port_out=(phase_acc>>24);
	//PORTD=next_reg;
	//counter++;
}

void timer_setup(void)
{
	//TIMSK0=(1<<OCIE0A);
	TCCR0A |= (1 << COM1A1);
	TCCR0A |= (1 << WGM11) | (0 << WGM00) | (1<<COM0A0) | (0<<COM1A0);
	TCCR0B |= (0 << CS10) | (0 << CS11) | (1 << CS12) | (0 << WGM02);
	OCR0A=16;
	//	OCR0B=64;
}
void pwm_setup(void)
{
	//TIMSK1=(1<<OCIE1A);
	TIMSK1=(1<<TOIE1);
	TCCR1A |= (1 << COM1A1);
	TCCR1A |= (1 << WGM11) | (1 << WGM10);
	TCCR1B |= (0 << CS10) | ( 1 << CS11) | (0 << CS12) | (0 << WGM13) | (0 << WGM12);
}

 

Last Edited: Mon. Jul 15, 2019 - 04:06 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have put a speaker, without filter, just with decoupling ca

 

What exactly are you talking about???!!  Didn't it have an amplifier like some old computer speakers or your stereo? 

 

You are trying to examine/verify (listen to) the filtered output for any problems!!  

 

 

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Mon. Jul 15, 2019 - 04:23 AM