Archived Forum

Have you read the description of how the sketch works?

If you do NOT want energy diversion, and simply want to understand how the PLL works, then look at http://openenergymonitor.org/emon/node/1377

yes step by step while reading this very nice blog but i still don't really feel very comfortable with all this.

For example this printing thing, I can't debug without printing :(

But thanks thanks thanks for the link it seems i could have saved some time...

[Do not double post. If your post is held for moderation, please wait for it to be attended to.]

*and i also don't understand what the code is doing here:

result = ADCL;
result |= ADCH<<8;

(...)

 sampleV = result;

Those are absolutely standard C language operators. Any moderately good tutorial on the C language will explain. Type "C operators" into your favourite search engine.

The ADC result is a 10-bit number but the interface to the ADC is through 8-bit registers, so you need to extract the low byte (ADCL) and the high byte (ADCH) and get them into the right place in your 16-bit variable, which is all the above code snippet is doing.  

Alternatively, you could let the compiler do the work for you and simply use "ADC" which is the symbol used to indicate the 16-bit result.  You can see in the code snippets below, this results in significantly less machine code.

uint16_t adc_value;

void setup() {  
  adc_value = ADCL;
  adc_value |= ADCH << 8;
}

produces:

000000a6 <setup>:
  a6:   80 91 78 00     lds     r24, 0x0078
  aa:   40 91 79 00     lds     r20, 0x0079
  ae:   34 2f           mov     r19, r20
  b0:   20 e0           ldi     r18, 0x00       ; 0
  b2:   90 e0           ldi     r25, 0x00       ; 0
  b4:   28 2b           or      r18, r24
  b6:   39 2b           or      r19, r25
  b8:   30 93 01 01     sts     0x0101, r19
  bc:   20 93 00 01     sts     0x0100, r18
  c0:   08 95           ret

while

uint16_t adc_value;

void setup() {  
  adc_value = ADC;
}

produces:

000000a6 <setup>:
  a6:    80 91 78 00    lds     r24, 0x0078
  aa:    90 91 79 00    lds     r25, 0x0079
  ae:    90 93 01 01    sts     0x0101, r25
  b2:    80 93 00 01    sts     0x0100, r24
  b6:    08 95          ret

The ADC result is a 10-bit number but the interface to the ADC is through 8-bit registers, so you need to extract the low byte (ADCL) and the high byte (ADCH) and get them into the right place in your 16-bit variable, which is all the above code snippet is doing.  

dBC,

Thanks for the explanation and the comparison. As an Electronics Tech, my specialty is hardware, so my C skills are not much more than beginner. Your answer helped me a lot.

Thanks again,

Bill

dBC,

oh thaaaank you very much !

I both got it and changed it :)

Robin,

thanks too, i promise i have worked a lot till now (i haven't done anything with c for the last 5 years so it's pretty chinese at this level)

Anyways I still get :

nan nan PLL is unlocked

The code that you gave me is working and the main difference I found was that part because it reads the result not with the ADC but with analogRead.

So ... still trying to find what is the useful thing I accidentally erased :)

Surely less code is better? Is there any reason by reading the high and low values is preferred over the single read? It is like this for older processors as the assembler instructions that do the actual read, IMO, look the same.

As I understand it, "analogRead" does much more than just transfer the values from the registers, that is unnecessary in this case. Speed is of the essence, which is why Martin did it the way he did. Maybe he didn't realise the 16-bit instruction existed.

analogRead() programs up the MUX, initiates a conversion, waits for the conversion to finish (typically about 110usecs with the most common clock settings), then fetches and returns the result.  It offers a completely synchronous interface to the ADC which is simple to understand but incredibly wasteful of clock cycles.  The CPU does nothing useful for the entire duration of the conversion.

The snippet of code above merely fetches the result of a previously completed conversion.  It takes just a handful of instructions,  and an even smaller handful if you use the 16-bit symbol rather than the two 8-bit symbols.

So it's not just that analogRead() does unnecessary stuff, it does destructive stuff if the results of a previous conversion were sitting there waiting to be fetched.  By the time analogRead() returns, the results of the previous conversion will be lost forever and replaced by a whole new conversion.

I think the whole point, which ioannamirto might have missed, is that towards the end of the Interrupt Service Routine, it sets the next input and starts the next conversion, then returns control to the main loop, which continues doing the things it does. When the conversion is complete 110 μs later, an interrupt is triggered, the main loop stops where it happens to be, and the ISR takes over, collects the data from the just-completed conversion and puts it where the main loop can reach it, then sets the next input etc.

[Edit: I have just checked - this is in Part 4 of the article about Martin's controller.]

Doing analogRead means that 110 μs of processing time is lost for each reading, and it is not possible to read samples continuously and send data by radio etc.

I agree with your comments Robert. I did not know a single read existed. The timing is controlled by the ISR. 

I have seen the common pattern of reading high and low in several sketches perhaps for the same reason. If there are no negative effects for a single read then surely this is better use of code.

But I am still a novice when it comes to Arduino.