understanding the code in depth

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

int main (void)
{ 

unsigned short oldphase, phase; 
unsigned int ADoutTemp;
unsigned int ADarray[3];

phase = 0;  // X, Y, Z:  0, 1, 2

oldphase = 0;

PORTD = 0;			// Initial output: Nada.

DDRD = 127U;		// Set port D lines to be outputs
					// Data Direction Register D 


////////////////////////////////////////////////////////////////////////////////

// ADC Setup

PRR &=  ~(_BV(ICF1));  //Allow ADC to be powered up

//ADC 3-5

ADMUX = 5;		// Channel 5 only

ADCSRA = _BV(ADEN) |_BV(ADPS2) | _BV(ADPS1) | _BV(ADPS0)  ;  // Enable ADC, prescale at 128.
					
ADCSRA |= _BV(ADSC);	// Start initial ADC cycle
			
			
////////////////////////////////////////////////////////////////////////////////
			
// Set up PWM output on 8-bit timer 0

TCCR0A = 163U;	

//TCCR0A:
// Bits 7,6:  10, for compare output mode:  Clear OC0A on Compare, set at top
// Bits 5,4:  10, for compare output mode:  Clear OC0B on Compare, set at top
// Bits 1,0:  11, (WGM01,WGM00): Fast PWM, with top value of 0xFF (WGM02 will be 0 for that mode)
// ==> TCCR0A is #10100011, or 163 decimal

TCCR0B = 1U;
	
//TCCR0B:
// Bits 7,6: 00, no force output compare
// Bit 3: 0		(WGM02, for Fast PWM, with top value of 0xFF)
// Bits 2,1,0:	001	Clock select  -- I/O clock, w/o divider

OCR0A = 0U;			//Start at zero percent duty cycle
OCR0B = 0U;			//Start at zero percent duty cycle
DDRD = _BV(PD5) | _BV(PD6);	// enable OC1A and OC1B as outputs, sums to #7 decimal. 


////////////////////////////////////////////////////////////////////////////////


// Set up PWM output on 16-bit timer 1

TCCR1A = 163U;	

//TCCR0A:
// Bits 7,6:  10, for compare output mode:  Clear OC0A on Compare, set at top
// Bits 5,4:  10, for compare output mode:  Clear OC0B on Compare, set at top
// Bits 1,0:  11, (WGM01,WGM00): Fast PWM, with top value of 0xFF (WGM02 will be 0 for that mode)
// ==> TCCR0A is #10100011, or 163 decimal

TCCR1B = 1U;
	
//TCCR0B:
// Bits 7,6: 00, no force output compare
// Bit 3: 0		(WGM02, for Fast PWM, with top value of 0xFF)
// Bits 2,1,0:	001	Clock select  -- I/O clock, w/o divider

OCR1A = 0U;			//Start at zero percent duty cycle
OCR1B = 0U;			//Start at zero percent duty cycle
DDRB = _BV(PB1) | _BV(PB2);	// enable OC1A and OC1B as outputs 

////////////////////////////////////////////////////////////////////////////////




// Set up PWM output on 8-bit timer 2, copied from timer 0 with appropriate mods.

TCCR2A = 163U;	

//TCCR2A:
// Bits 7,6:  10, for compare output mode:  Clear OC0A on Compare, set at top
// Bits 5,4:  10, for compare output mode:  Clear OC0B on Compare, set at top
// Bits 1,0:  11, (WGM01,WGM00): Fast PWM, with top value of 0xFF (WGM02 will be 0 for that mode)
// ==> TCCR0A is #10100011, or 163 decimal

TCCR2B = 1U;
	
//TCCR2B:
// Bits 7,6: 00, no force output compare
// Bit 3: 0		(WGM02, for Fast PWM, with top value of 0xFF)
// Bits 2,1,0:	001	Clock select  -- I/O clock, w/o divider

OCR2A = 0U;			//Start at zero percent duty cycle
OCR2B = 0U;			//Start at zero percent duty cycle
DDRD |= _BV(PD3);	// enable OC2B as output
DDRB |= _BV(PB3);   // enable OC2A as output


////////////////////////////////////////////////////////////////////////////////

											
for (;;)  // main loop										
{

if ((ADCSRA & _BV(ADSC)) == 0)		// If ADC conversion has finished
{	

	ADoutTemp = ADCW;			// Read out ADC value	
	
/* We are using *ONE* ADC, but sequentially multiplexing it to sample
the different input lines, using the variable 'phase' to represent
which axis we are looking at.
	phase values for X, Y, Z:  0, 1, 2
	*/
	
	if (++phase > 2)
		phase = 0;
	 
	 ADMUX = (5 - phase);
	   //Phase == 0: ADC 5
	   //Phase == 1: ADC 4
	   //Phase == 2: ADC 3
	   
    ADCSRA |= _BV(ADSC);	// Start new ADC conversion
	
	ADarray[oldphase] =  ADoutTemp >> 2; // Convert 10-bit result to 8-bit result,
 										 // store at appropriate place in result array.
	oldphase = phase;
}



if (ADarray[0] >= 128){
   DDRD &= (0xFF -  _BV(PD6));
   OCR0A = 0;
   
   DDRD |= _BV(PD5) ;
   OCR0B = (unsigned short) (2*(ADarray[0] - 128));
   }
else {
   DDRD |= _BV(PD6) ;
   OCR0A = (unsigned short)  (2*(127 - ADarray[0]));
   
   DDRD &= (0xFF -  _BV(PD5));
   OCR0B = 0;
   }




if (ADarray[1] >= 128){
   DDRB &= (0xFF -  _BV(PB1));
   OCR1A = 0;
   
   DDRB |= _BV(PB2);
   OCR1B = (unsigned short) (2*(ADarray[1] - 128));
   }
else {
   DDRB |= _BV(PB1);
   OCR1A = (unsigned short)  (2*(127 - ADarray[1]));
   
    DDRB &= (0xFF -  _BV(PB2));  
   OCR1B = 0;
   }




if (ADarray[2] >= 128){

   DDRB &= (0xFF -  _BV(PB3));  
   OCR2A = 0;
   
   DDRD |= _BV(PD3);
   OCR2B = (unsigned short) (2*(ADarray[2] - 128));
   }
else {
   
   DDRB |= _BV(PB3); 
   OCR2A = (unsigned short)  (2*(127 - ADarray[2]));

 DDRD &= (0xFF -  _BV(PD3));
   OCR2B = 0;
   }




}// End main loop


	return 0;
}

please explain me this code..in depth...

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

Quote:

please explain me this code.

Is that not what the comments are doing already?

(the fact that the author appears to have never heard of indentation makes it very difficult to follow so I'd start by indenting it correctly which may help your ability to read it).

BTW constructs such as:

DDRD &= (0xFF -  _BV(PD3)); 

instead of the rather more obvious:

DDRD &= ~_BV(PD3); 

suggest this may have been written by a beginner.

BTW that unusual construct makes it easy to find the source of this code on Google. It is in a .zip file linked from this page:

http://www.evilmadscientist.com/...

(shame about the pictures!) That page explains something about its operation.

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

Quote:
suggest this may have been written by a beginner.
Not to mention the use of indecipherable decimal constants to set bits.

Regards,
Steve A.

The Board helps those that help themselves.

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

Quote:
the fact that the author appears to have never heard of indentation makes it very difficult[...]

I have, in fact, heard of indentation. That's a pretty low blow, don't you think?

But that's not the most important thing going on here. What's really important is that you-- a moderator of the site, for goodness sake! --are once again seizing an off-hand opportunity to make ad hominem attacks about someone that you don't actually know.

This is quite simply indefensible. I've known for years that the AVR Freaks community has a bit of a toxic side to it; it's why I don't hang out here. And your comments here are yet another good reminder that this continues to be the case. There's still an apparent eagerness to transition directly from a (sometimes poorly phrased, but yet earnest) technical question to personal attacks.

People come here for help, and quite often we either insult them, or the thing that they asked the question about (or in this case, the person who wrote the thing that they're asking about). There are thousands of examples of this kind of behavior here. I am not ashamed about my code, but I am truly, deeply ashamed to be a member of the community of AVR developers when this is how we treat people.

It might be a kinder place here if the policy once suggested by a certain clawson were actually to be implemented:

Quote:
[...] ad hominem attacks against any member here will not be tolerated and action will be taken by anyone doing it and ALL the moderators and Admins agree on this.

Now, as to the point: If I had known four years ago that anyone would have been scrutinizing this code far in the future, I would have cleaned it up considerably, both in terms of indentation, constants, and bitwise logic operations. As it was at the time, I thought that it was better to post a functional code example-- ugly or not --in case someone else might find it helpful someday. If you disagree, well, fair enough. Heck: I apologize that its ugliness offends you. But be certain of this: its ugliness does not justify your behavior in the least.

Quote:
(shame about the pictures!)
If you have a suggestion about how to improve my pictures in the future, please do let me know. But just trashing them without indicating what is wrong is, well, apparently par for this sad, sad course.

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

Quote:

That's a pretty low blow, don't you think?

No - you have written the code to be virtually unreadable. That's fine for something you knock up in your back bedroom and don't plan to distribute. For anything that is to be published you should have the next reader in mind and be trying to help them. Your code fails.

This is not an ad hominem atack. I said nothing about the author only about the very poor looking code. The evidence is clear it is poor code. When beginners go searching on the internet for code to use they can never tell the provenance. They can be guided as to whether something "looks good" by those who have more experience. I do - I would not recommend your code to anyone in its current state. If it's full of basic misunderstandings how can anyone who just found it in Google know that it isn't equally faulty in operation and waste a lot of time on an unknown?

I'm happy to recommend well written and documented code like Peter Fleury or Pascal Stang library code to beginners as you can instantly tell by looking that it is quality code. If there's any doubt then code is best avoided unless someone can give a personal recommendation that it works well. Too much time is wasted trying to patch up "unknown" code found on the internet.

BTW I actually thought the comments were pretty good up to a point but the whole code really fell down when the last three main statement blocks, that are performing the majority of the work, fail to have any comment whatsoever as to what the purpose of what is being done is. That was also a real let down.

Hence I warned the OP about "questionable" looking code - because it is.

Quote:

(shame about the pictures!)

In my browser at least there were a load of plain white rectangles - no pictures visible whatsoever. Hence the comment "shame about the pictures". If I could see them I'm sure they'd look lovely.

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

Quote:
This is not an ad hominem atack. I said nothing about the author only about the very poor looking code.

Here is what you said: "the author appears to have never heard of indentation." That is insulting the author, and not the code. This is a classic example of an ad hominem attack.

Trash code all you like. I don't have a problem with that. I did not ask you to recommend my code, and I even apologized. But again: its ugliness does not justify your behavior in the least.

When you use ad hominems-- when you attack the person not the code --you drag this forum through the mud. As a moderator of this forum, and one who has publicly spoken out against ad hominem attacks here, you should know better.

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

Quote:

That is insulting the author,

No it is not "insulting". It is a true statement of fact. Looking at the code above it is pretty clear that whoever wrote it knew little/nothing about indentation in C. The idea is to give a view of structure and execution flow. That cannot be determined by the code show above so clearly the author did not know about the importance of indentation when he wrote it. I didn't say he was an idiot because of this or use any other insulting term. I simply, said it looked like he'd never heard of indentation. Surely that is a true statement of fact?

I still do not see how you could possibly interpret that statement as a direct insult. Admit it yourself - you clearly did not understand indentation when that code was written did you?