AVR Freaks

@DocJC, no I cannot control it by the host system, nor by the BIOS, that is why I have to walk the alternative route. This method is not used before as I am aware of (otherwise I would love to copy paste ;-) ), but it is done more, only in different ways. The problem here is that the host system is checking in firmware closed source the fan speed.

@Kleinstein, I am aware that this looks/is software PWM. I have been searching the net a lot, but did not come across real DDS.

Would love to read something about this (even if it is just to get an idea). Basically I was "scared" to dive in Jesper etc, because it is, ASM and that would be harder to me.

Concerning 16bit is correct, this would be enough. 

Furthermore. If you talk about 1ISR long, you probably mean 1 timer_tick?

This I do not understand: "Thus the lookup take usually used in DDS is to be replaced with something like checking for an "overflow"."

But if I understand you correct then underneath code would do the trick:

ISR(xxx_vect){
    static uint8_t timer_tick;
    timer_tick++;                                                   //increment the clock tick
    if(timer_tick % freq1 == 0){do_this("do nothing -> 0");}        //do every X ticks
    if(timer_tick % (freq1+1) == 0){do_that("give pulse -> 1");}    //do every X+1 ticks
    
    if(timer_tick % freq2 == 0){do_this("do nothing -> 0");}        //do every Y ticks
    if(timer_tick % (freq2+1) == 0){do_that("give pulse -> 1");}    //do every Y+1 ticks
}

Or would you prefer:

ISR(xxx_vect){
    static uint8_t timer_tick1;
    timer_tick1++;                                                   //increment the clock tick
    if(timer_tick % freq1 == 0){do_this("do nothing -> 0");}        //do every X ticks
    if(timer_tick % (freq1+1) == 0){do_that("give pulse -> 1" & reset clock1 0);}    //do every X+1 ticks
    
    static uint8_t timer_tick2;
    timer_tick2++;      
    if(timer_tick % freq2 == 0){do_this("do nothing -> 0");}        //do every Y ticks
    if(timer_tick % (freq2+1) == 0){do_that("give pulse -> 1" & reset clock2 0);}    //do every Y+1 ticks
}

Forgot to mention, I am looking for a Atmega only solution, so not a specific chip to create the wave.

Thank you, makes it clearer.

Corrected this already. Indeed its just a timer tick. Renamed it to avoid any further confusion. 

void checkFreq() {														// Function to be triggered if frequency needs to be adjusted during runtime
	freqCurrent = (((Average_RPM)+(0X3C/2))/0X3C);							// FAN RPM
		if (abs(freqCurrent-frequency)>freqTolerance){					// if new value exceeds tolerance
		frequency = freqCurrent;										// new frequency
		
		temp_var = (((0x8000 * (float)frequency)+((float)samplerate/2))/(float)samplerate);
		phase_inc01 = (uint16_t)temp_var;								// Calculate 1 period versus complete samplerate
	}
}

As seen above I have adjusted the calculation. If I used a decimal instead of the 0x8000 then it wars working. If I used a hex it was not. Casting all to float made it working. (Hope this is what you ment?).

Furthermore I understand the part most significant bit, what I do not understand is that I see my : phase_acc01+=phase_inc01; reachting a value of 65k before retruning to +0.

So why do I need to do the calculation(in checkFreq) and comparison(in timer) to 0x8000 and not to 0xFFFF if that is what the timer runs to until is starts again at 0.

Or is the purpose to reset manually to 0 like with the 1000ms timer?

I am sorry I did not respond to your question, my apologies.

To describe:

My pulse is a Tacho pulse from a fan (the AVR needs to recreate one). The original pulse is created by an internal transistor with the wire which is connected to the AVR as open collector.

The pulse will come every round once like a hall sensor.

The pulse I want to create is likewise. So the main part of the frequency is 0 and only 1 pulse needs to be given every period.

Visualized:

______________________-______________________-______________________-

Where the underscore is NO pulse and the dash is a pulse.

As I just explained I do not want a 50:50 signal. That is why I was focusing on 65535 instead on 32k.

I now know about the bitwise AND.

This is checking is the most significant bit from BOTH sides of the "&" are set, if they are then port high, otherwise in the time-frame clear.

(Hope I finally understood...)

Simple answer, because I do not know. But I agree your way is better, probably more efficient.

Yep had read that somewhere.

Bitwise operations is not my daily job, so just had a quick google and understand what you are doing.

http://stackoverflow.com/questio... :)

Furthermore I also noticed you removed the +0.5. Besides you noted earlier that this was not my problem you did not explain why I should not need it?

Why do you use 100000UL and not the "samplerate"

I will read that tutorial before changing.

You mentioned that before. Let me first conquer the traps with interrupts.

You told me before, but I need to know how the mainboard reacts on a squarewave instead of a pulse. Normally the hall sensor in the fan will just give a short pulse and not a square wave.

If it accepts the square I am fine with it.

Did not define it in your mentioned way, but as a variable. See your point and change it. I only wanted to have a simple place instead I change it, so when I change it, it would change all at once. Instead of accidentally missing a variable.

Okey, I fully understand what you are saying. 

I need to have max 15k rpm (which is 500Hz)

Furthermore I need 1Hz for the timer (seconds), preferably with 1 decimal in case I need to increase the resolution. So 1,1 second or 0.5 second (500ms) ->10Hz

Reread the manual:

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

Timer Resolution = (1 / Input Frequency)
Timer Resolution = (1 / 10) = 0.1 seconds
Timer Resolution = (1 / 500) = 0.002 seconds
Timer Resolution = (1 / 100000) = 0.00001 seconds -> current setup
Timer Resolution = (1 / 1000) = 0.001 seconds -> current setup

Now calculate target timer count:

Target Timer Count = (1 / Target Frequency) / (1 / Timer Clock Frequency) - 1
                   = (1 / 1000) / (1 / 20000000) - 1
                   = 0.001 / 0.00000005 - 1
                   = 20000 - 1
                   = 19999

So without prescale I need 19999 ticks for 1000Hz -> 19999*1000=19999000

For 100000Hz this would be 199. To get 1 second ->   199*100000=19900000

Calculate correct prescaler

Target Timer Count = (Input Frequency / Prescale) / Target Frequency - 1

Prescaler Value | Target Timer Count

              1 |  19999
              8 |  2499
             64 |  311.5
            256 |  77.125
           1024 |  18.53

So based on the 1000Hz freq I choose to get a nice 0.001 resolution (which I think is accurate enough to make me 500Hz) I can use a prescaler of 8.

   TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode
   OCR1A   = 2499; // Set CTC compare value to 1000Hz at 20MHz AVR clock, with a prescaler of 8

   TCCR1B |= (| (1 << CS11)); // Start timer at Fcpu/8

Does this makes sence?

Have fun while watching F1

Found my problem (I think). The piece of code which was handling the external interrupts was in the initial code responding to a high and a low, causing it to 2 times count 1 pulse.

After adjusting it with the underneath code it now gives sensible values. My fan reports 293~294~295. Which is something I am happy with. Translated to RPM this is 8850.

Now I can continue to test the wave generation to the MB. Very curious if it works... Fingers crossed.

	if(changedbits & (1 << PA0))
	{
		/* PCINT0 changed */
		/* LOW to HIGH pin change */
		if( (PINA & (1 << PA0)) == 1 )
		{
			// Only on high add count
			rpmcount[0]++;
		}
	}

Update: tested with above addition and RPM is OK. Next step was to test the wave generation (Square wave 50:50). Amazingly this work directly.

At the moment my AVR is just a sandbox. Next step now is to add the ratio calculation.

Will test that functionality tomorrow. But besides that I am already quite happy with the current result. When tomorrow is a success I can finally finalize my print and get it made.

Order the necessary stuff and then hopefully this weekend I have my server running (nice and smooth AND SILENT).

Kartman (and others) if you have any comments on my code please feel free.

Removed due to changes made to source -> scroll down for a later post that holds actual code!

This will be my last reply in this topic. I have finished my code and it is working as expected. With this code you are able to control (manipulate) the TACH output of 5 different fans and adjust that output by a given ratio. Then that new frequency is used to create a square wave to feed back to the mainboard. The mainboard picks this signal up and you can display/adjust whatever you want.

Unfortunately this solution did not solve my initial problem. For this I will start a new project.

I would like to thank especially Kartman for his patience and help during this project.

(Code written for an AVR644)

#ifndef F_CPU
#define F_CPU 20000000UL
#endif

# define SAMPLERATE 50000UL
# define USART_BAUDRATE 9600

// INRATIO:OUTRATIO
# define INRATIO 1
# define OUTRATIO 1

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include <util/atomic.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include "uart.h"

uint16_t rpmcount[5];						// default start counting at 0 for fan 1
uint16_t rpmcalc[5];
volatile uint8_t portbhistory = 0xFF;		// default is high because the pull-up
char buffer_var[10];						// buffer used for usart translations

volatile uint8_t flag = 0;
volatile uint16_t phase_inc[5];

uint32_t timer_tick=0;
uint16_t phase_acc[5];

// variables for freq monitoring
uint16_t frequency[5];
uint8_t freqTolerance = 2;					//adjust this to increase/decrease stability of frequency measurement


uint16_t phase_inc_var = 0;

ISR (PCINT0_vect)
{
	uint8_t changedbits;

	changedbits = PINA ^ portbhistory;
	portbhistory = PINA;

	if(changedbits & (1 << PA0))
	{
		/* PCINT0 changed */
		/* LOW to HIGH pin change */
		if( (PINA & (1 << PA0)) == 1 )
		{
			// Only on high add count
			rpmcount[0]++;
		}
	}

	if(changedbits & (1 << PA1))
	{
		/* PCINT1 changed */
		/* LOW to HIGH pin change */
		if( (PINA & (1 << PA1)) == 1 )
		{
			// Only on high add count
			rpmcount[1]++;
		}
	}

	if(changedbits & (1 << PA2))
	{
		/* PCINT2 changed */
		/* LOW to HIGH pin change */
		if( (PINA & (1 << PA2)) == 1 )
		{
			// Only on high add count
			rpmcount[2]++;
		}
	}
	if(changedbits & (1 << PA3))
	{
		/* PCINT3 changed */
		/* LOW to HIGH pin change */
		if( (PINA & (1 << PA3)) == 1 )
		{
			// Only on high add count
			rpmcount[3]++;
		}
	}
	if(changedbits & (1 << PA4))
	{
		/* PCINT4 changed */
		/* LOW to HIGH pin change */
		if( (PINA & (1 << PA4)) == 1 )
		{
			// Only on high add count
			rpmcount[4]++;
		}
	}
}

void INPUT_Init()
{
	DDRA &= ~((1 << DDA0) | (1 << DDA1) | (1 << DDA2) | (1 << DDA3) | (1 << DDA4)); // Clear the PA0, PA1, PA2 pin 
	PORTA |= ((1 << PA0) | (1 << PA1) | (1 << PA2) | (1 << PA3) | (1 << PA4));      // turn On the Pull-up
	
	PCICR |= (1 << PCIE0);     // set PCIE0 to enable PCMSK0 scan
	PCMSK0 |= (1 << PCINT0);   // set PCINT0 to trigger an interrupt on state change
}

void OUTPUT_Init()
{
	//set port/pin  mode
	DDRC |= ((1<<DDC0) | (1<<DDC1) | (1<<DDC2) | (1<<DDC3) | (1<<DDC4));
	DDRD |= (1<<DDD3);          // set LED pin PD1 to output
}

ISR (TIMER1_COMPA_vect)
{
	timer_tick ++;
	if (timer_tick  == SAMPLERATE){
		// Trigger frequency calculation
		flag=1;
		memcpy( rpmcalc, rpmcount, sizeof(rpmcount) );	// copy contents of rpmcount so interrupt can stay running							
		memset(rpmcount, 0, sizeof rpmcount);			// Reset array "rpmcount"
		timer_tick = 0;									// Reset timer
	}
	
	// All individual fan accumulators
	phase_acc[0]+=phase_inc[0];
	// copy msb to port bit
	if (phase_acc[0] & 0x8000){
		PORTC |= (1<<PC0);			// Set port bit
	}
	else
	{
		PORTC &= ~(1<<PC0);			// Clear port bit
	}
	
	phase_acc[1]+=phase_inc[1];
	// copy msb to port bit
	if (phase_acc[1] & 0x8000)
	{
		PORTC |= (1<<PC1);			// Set port bit
	}
	else
	{
		PORTC &= ~(1<<PC1);			// Clear port bit
	}
	
	phase_acc[2]+=phase_inc[2];
	// copy msb to port bit
	if (phase_acc[2] & 0x8000)
	{
		PORTC |= (1<<PC2);			// Set port bit
	}
	else
	{
		PORTC &= ~(1<<PC2);			// Clear port bit
	}
	
	phase_acc[3]+=phase_inc[3];
	// copy msb to port bit
	if (phase_acc[3] & 0x8000)
	{
		PORTC |= (1<<PC3);			// Set port bit
	}
	else
	{
		PORTC &= ~(1<<PC3);			// Clear port bit
	}
		
	phase_acc[4]+=phase_inc[4];
	// copy msb to port bit
	if (phase_acc[4] & 0x8000)
	{
		PORTC |= (1<<PC4);			// Set port bit
	}
	else
	{
		PORTC &= ~(1<<PC4);			// Clear port bit
	}

}
void timer_init()
{	
	TCCR1B |= (1 << WGM12);					// configure timer1 for CTC mode
	TIMSK1 |= (1 << OCIE1A);				// enable the CTC interrupt
	//Target Timer Count = (Input Frequency / Prescale) / Target Frequency - 1
	OCR1A = (F_CPU/SAMPLERATE)-1;			// set the CTC compare value
	
	TCCR1B |= (1 << CS10);					// enable timer with NO prescaler
}

void checkFreq() {
	// temporary display current rotations per 1000ms (in pulses)
	for (int y=0; y<=4; y++){
		itoa(rpmcalc[y], buffer_var, 10);
		uart_puts(buffer_var);
		uart_puts("\r\n");
	}

	// Calculation new FAN delta phase (based on "current real rotations")
	for (int y=0; y<=4; y++){
		if (abs(rpmcalc[y]-frequency[y])>freqTolerance){					// if new value exceeds tolerance
			frequency[y] = rpmcalc[y];										// new frequency
			phase_inc[y] = (((uint32_t)(frequency[y])*OUTRATIO)<<16) / (SAMPLERATE*INRATIO);		// Calculate 1 period versus complete sample rate
		}
	}
	memset(rpmcalc, 0, sizeof rpmcalc);
}

int main(void)
{
	// Initialize program parameters/functionalities
	
	cli();													// Disable interrupt
	timer_init();											// Init Timer
	INPUT_Init();											// Init Input ports
	uart_init( UART_BAUD_SELECT(USART_BAUDRATE,F_CPU) );	// Init UART functionality
	OUTPUT_Init();
	
	// Local Variables used
	memset(rpmcount, 0, sizeof rpmcount);					// Initialize first time rpmcount
	checkFreq();											// Make sure all fans have RPM on start	
	
	// Start Program
	sei();													// Enable interrupt
	
	while (1) {
		/* Wait for the ISR to set the 
		* flag; reset it before 
		* taking any action. */ 
		if (flag==1) 
		{ 	
			flag = 0;
			/* Perform the required action here */
			checkFreq();	
		}
	}
}

Seems that the real problem is not directly the logic in the IFs...

I moved the rpmcount before all that and connected my fan to PA3 (or an other PA <> PA0). Seems no interrupt is triggered at all on these ports.

Kinda lost at the moment.

Found the problem, after thoroughly investigating the manual (again):

	This only enabled the PA0
	PCMSK0 |= (1 << PCINT0);   // set PCINT0 to trigger an interrupt on state change
	
	So it should have been:
	PCMSK0 |= ((1 << PCINT0) | (1 << PCINT1) | (1 << PCINT2) | (1 << PCINT3) | (1 << PCINT4)); 

ISR code (working as far as I can see):

ISR (PCINT0_vect)
{
	uint8_t changedbits;

	changedbits = PINA ^ portbhistory;
	portbhistory = PINA;

	if((changedbits & (1 << PA0))&&(~PINA & (1 << PA0)))
	{
		/* PCINT0 changed & high */
		rpmcount[0]++;
	}
	if((changedbits & (1 << PA1))&&(~PINA & (1 << PA1)))
	{
		/* PCINT1 changed */
		rpmcount[1]++;
	}
	if((changedbits & (1 << PA2))&&(~PINA & (1 << PA2)))
	{
		/* PCINT2 changed */
		rpmcount[2]++;
	}
	if((changedbits & (1 << PA3))&&(~PINA & (1 << PA3)))
	{
		/* PCINT3 changed */
		rpmcount[3]++;
	}
	if((changedbits & (1 << PA4))&&(~PINA & (1 << PA4)))
	{
		/* PCINT4 changed */
		rpmcount[4]++;
	}
}

I think that dropped of while modifying the code. Earlier post show the comment. But indeed should be in there.

Totally agree, I already modified it with my current code.

A write-out to understand what you propose:

PA3 gets high (low by pullup):

rising_edges = ~porthistory & our_pina;
00000000 & 11110111 = 00001000

if (rising_edges & (1<<PA3)) //if rising edge on PA3
00001000 & 00001000 = true

PA2 gets high (low by pullup):

rising_edges = ~porthistory & our_pina;
00001000 & 11110011 = 00000100

if (rising_edges & (1<<PA2)) //if rising edge on PA2
00000100 & 00000100 = true

PA3 gets low (high by pullup):

rising_edges = ~porthistory & our_pina;
00001100 & 11111011 = 00001000 ->>> should be 00000000

if (rising_edges & (1<<PA3)) //if rising edge on PA3
00001000 & 00001000 = true

Can it be that your logic contains a mistake? When I have a change on the pin high to low. It also fires the PA3 code. See above example. Might be that I made a mistake.

Current code (with the 1 time PIN read):

ISR (PCINT0_vect)
{
	uint8_t changedbits;
	uint8_t our_pina;
	
	our_pina = PINA; //read once
		
	changedbits = our_pina ^ portbhistory;
	portbhistory = our_pina;
	
	/* PCINT0 changed & high */
	if((changedbits & (1 << PA0))&&(~our_pina & (1 << PA0)))
	{
		rpmcount[0]++;
	}
	/* PCINT1 changed & high */	
	if((changedbits & (1 << PA1))&&(~our_pina & (1 << PA1)))
	{
		rpmcount[1]++;
	}
	/* PCINT2 changed & high */
	if((changedbits & (1 << PA2))&&(~our_pina & (1 << PA2)))
	{
		rpmcount[2]++;
	}
	/* PCINT3 changed & high */
	if((changedbits & (1 << PA3))&&(~our_pina & (1 << PA3)))
	{
		rpmcount[3]++;
	}
	/* PCINT4 changed & high */
	if((changedbits & (1 << PA4))&&(~our_pina & (1 << PA4)))
	{
		rpmcount[4]++;
	}
}

After some time I restarted my project (converted to xmega), however I am running in a more general problem (not related to x/atmega).

What I see is when running the code with a 4000RPM spec fan that the rpmcount is outputting 127 per second (127*60 = 7620 RPM). This delivers 7620 RPM. Of course way to high, almost double of the fan spec.

Already spend a day looking and recalculating, but so far I havent found the problem, other than the timing issue, the code is running fine...

(Btw I am looking into your last advice for simplifying the code, will come back on that one)

Your suggestion with simplification now also works in my code (result is still off but values are consistent with my other version)