Trying to read from ADC channels with attiny85

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

Hello everyone.

 

I'm having some problems trying to read alternately the different ADC channels of an attiny85. In essence, the problem I have is when trying to manipulate which channel I am reading at a given moment and the transition to the next channel I want to read.

 

I have searched numerous threads and tried different proposed solutions but none has been successful, until now. Therefore, I think it's time to see if someone can guide me a little. I will try to be as clear and simple as possible.

 

What do I have to do: given an ISR function (ADC_vect), depending on the ADC channel I am reading, I must call another function "x_channel". After calling that function "x_channel" I must change the channel that I am reading by another, in order to alternate between the different channels so that I can read at some point all that are useful. In other words, the algorithm should read ADC0 in one call, ADC2 in the next, ADC3 in the later and then reread ADC0 to repeat the loop...

 

Channels that I should read:
- ADC0 = PB5 => (MUX = 00)
- ADC2 = PB4 => (MUX = 10)
- ADC3 = PB3 => (MUX = 11)

 

Note: ADC1 I don't need to read it.

 

Pseudocode:

ISR (ADC_vec)
{
  if (ADC_source is PB5)
  {
    call ADC0 channel function;
    ADC_source = PB4;
  } else {
    if (ADC_source is PB4)
    {
      call function channel ADC2;
      ADC_source = PB3;
    } else {
      if (ADC_source is PB3)
      {
        call function ADC3 channel;
        ADC_source = PB5;
      }
    }
  }
}

where ADC_source can be implemented through ADMUX, as I understand it.

 

My intent:

ADMUX is initialized as:

ADMUX = (1 << ADLAR) | (1 << MUX1) | (1 << MUX0);

Code:

ISR(ADC_vect)
{
  uint8_t sample_value;
  sample_value = ADCH;
    
  if (ADMUX & 0x00) // sample from PB5
  {
    TurnOnLED_ADC0(); // call function for ADC0
    ADMUX = (1 << MUX1) | (0 << MUX0); // Swap ADMUX to 10
  } else {
    if (ADMUX & 0x02)
    {
      TurnOnLED_ADC2(); // call function for ADC2
      ADMUX = (1 << MUX1) | (1 << MUX0); // Swap ADMUX to 11
    } else {
      if (ADMUX & 0x03)
      {
        TurnOnLED_ADC3(); // call function for ADC3
        ADMUX = (0 << MUX1) | (0 << MUX0); // Swap ADMUX to 00
      }
    }
  }
}

 

Results: until now the algorithm does not respond as expected. The biggest difficulty is that when debugging it I can not distinguish if the error is in the way of comparing ADMUX to know if it is reading a given channel, either the modification of ADMUX is being done correctly, or even if both proposals are wrong.

 

Any opinion about it? Thank you very much in advance.

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

When you change channels you may need to throw away the first reading.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

 

if (ADMUX & 0x00) // sample from PB5

This test will never be true

    if (ADMUX & 0x02)
    {
    } else {
      if (ADMUX & 0x03)

I'm sure this isn't what you want, since when bit 1 is set, both conditions will be true but only the first will "fire"

 

My guess is you want your if statements to be in the form

if ((ADMUX & 0x0F) == 0x00)  // same for 0x02 and 0x03

 

Also, what you are trying to do would be cleaner with a 'switch' statement.

 

Last Edited: Tue. Aug 7, 2018 - 04:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

deleted because i had the same points as the previous post.

Last Edited: Tue. Aug 7, 2018 - 04:58 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

js wrote:

When you change channels you may need to throw away the first reading.

Yes, I do that, only I did not include it in the code to simplify it. Whenever this function is called, the first reading is discarded.

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

kk6gm wrote:
Also, what you are trying to do would be cleaner with a 'switch' statement.
You're right. Thank you very much for the suggestion, now I will incorporate it.

 

This is how the code remained after the proposed modifications. It still does not work as it should, do you think I missed something?

ISR(ADC_vect)
{
  static bool_t first_sampling = false;
  uint8_t sample_value;
  sample_value = ADCH;
  
  
  if (first_sampling == true)
  {
    first_sampling = false;
  }
  else
  {
    switch (ADMUX & 0x0F)
    {
      case (0x00):   // ADC0 MUX[1:0] == 00
        //TurnOnLED_ADC0();
        // Update ADMUX to ADC2
        ADMUX = (1 << MUX1) | (0 << MUX0);
        break;
      case (0x02):  // ADC2 MUX[1:0] == 10
        //TurnOnLED_ADC2();
        // Update ADMUX to ADC3
        ADMUX = (1 << MUX1) | (1 << MUX0);
        break;
      case (0x03):  // ADC3 MUX[1:0] == 11
        //TurnOnLED_ADC3();
        // Update ADMUX to ADC0
        ADMUX = (0 << MUX1) | (0 << MUX0);
        break;
      default:
        break;
    }
    first_sampling = true;
  }
}

 

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

elbraca wrote:
My intent: ADMUX is initialized as: ADMUX = (1 << ADLAR) | (1 << MUX1) | (1 << MUX0);

...but now you are >>not<< using ADLAR, so you end up with a 2-bit result:

elbraca wrote:
ADMUX = (0 << MUX1) | (0 << MUX0);

 

We don't see all the code.  You need to start the next conversion in the ISR -- unless there is another trigger mechanism.  (and I hope it is not "free running"...)

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Aug 7, 2018 - 04:16 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You forget to mention the MOST important point...can you even read ONE channel properly?

 

Do not bother or worry about changing channels until you get ONE working properly.  Focus your efforts on reading ADC0, when it works 100%, move on.

When in the dark remember-the future looks brighter than ever.

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

Also, why are you essentially toggling first_sampling each time through the ISR?

 

And

if (first_sampling == true)

 is unnecessary.  This does the same thing and reads cleaner:

if (first_sampling)

EDIT: Typical first-time-thru code looks more like this:

static bool first_time_thru = true;
...
if (first_time_thru)
  first_time_thru = false;
else
{
    // code for all the other times thru
    // DON'T mess with first_time_thru anymore here
}

 

Last Edited: Tue. Aug 7, 2018 - 04:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'll go in parts to try not to skip anything. First of all, I'm working on a code that someone else did. Many things are not optimized, but it's written so that anyone with little experience can understand it relatively quickly. That's why you can find some redundancies. Some things I'm polishing but the first goal is to make the algorithm work as expected, then I can optimize it.

 

theusch wrote:
You need to start the next conversion in the ISR -- unless there is another trigger mechanism.  (and I hope it is not "free running"...)
 The second most important thing is that it is free running. There's an infinite loop in the main function.

 

avrcandies wrote:
You forget to mention the MOST important point...can you even read ONE channel properly?   Do not bother or worry about changing channels until you get ONE working properly.  Focus your efforts on reading ADC0, when it works 100%, move on.
The original code read from ADC2 and ADC3 and swapped between those two channels. It worked perfectly and this was done in the following way:

ISR(ADC_vect)
{
  static bool_t first_sampling = false;
  uint8_t sample_value;
  sample_value = ADCH;

  if (first_sampling == true)
  {
    first_sampling = false;
  }
  else
  {
    if (ADMUX & 0x01)   // MUX[0:0] == 1
    {
      turnOnLED_ADC3();
    }
    else    // MUX[0:0] == 0
    {
      turnOnLED_ADC2();
    }
    // Switch the ADC input channel
    ADMUX ^= (1 << MUX0);   // toggle between MUX[1:0] = 11 and MUX[1:0] = 10
    first_sampling = true;
  }
}

My modification is to add a third reading channel, using ADC0. For that, part of the code just described does not work. Therefore, I tried the following solution, which until now is still not working. To debug it, I thought I could "do nothing" when reading ADC0, so if everything is fine, you would expect the same behavior you had before the modification. But that is not what is happening.

ISR(ADC_vect)
{
  static bool first_sampling = true;
  uint8_t sample_value;
  sample_value = ADCH;

  if (first_sampling)
  {
    first_sampling = false;
  }
  else
  {
    switch (ADMUX & 0x0F)
    {
      case (0x00):   // ADC0 MUX[1:0] == 00
        // Do Nothing Here...
        // Update ADMUX to ADC2
        ADMUX = (1 << MUX1) | (0 << MUX0);
        break;
      case (0x02):  // ADC2 MUX[1:0] == 10
        turnOnLED_ADC2();
        // Update ADMUX to ADC3
        ADMUX = (1 << MUX1) | (1 << MUX0);
        break;
      case (0x03):  // ADC3 MUX[1:0] == 11
        turnOnLED_ADC3();
        // Update ADC Source to ADC0
        ADMUX = (0 << MUX1) | (0 << MUX0);
        break;
      default:
        break;
    }
  }
}

kk6gm wrote:
EDIT: Typical first-time-thru code looks more like this:
 I took the opportunity to modify what you mentioned. Thank you.

Last Edited: Tue. Aug 7, 2018 - 06:10 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

elbraca wrote:
Therefore, I tried the following solution, which until now is still not working.

Define "not working".  What is the refer3ence voltage?  What are the signal voltages >>right on the AVR pins<< ?  How many counts do you expect?  How many counts are you getting?

 

I said nothing about efficiency.  [That said, in a full application it is rarely a good thing to call processing functions from an ISR.  In fact, if they take e.g. longer than 100us or so you run the risk of starving interrupts.]  But I DID say that with your code you are getting a 2-bit result, because you are trashing ADLAR.

 

Simple to read?  I'd structure the "simple to read" quite differently.

 

IMO/IME using free-running is rarely useful, especially with multi-channel work.  It will only make your head hurt.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Aug 7, 2018 - 07:13 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The poster here https://www.avrfreaks.net/commen... wanted to go through all the channels, and unrolled into a switch ().  Excerpt:

...
switch(channelADC)
	{
		case 0:
		PORTB ^= (1 << GREEN_LED); // Toggle the LED
		ADCSample[0]=ADCResult;
		channelADC++;
		break;

		case 1:
		ADCSample[1]=ADCResult;
		channelADC++;
		break;
...

Personally, I find repetitive coding isn't any fun, and is error prone.  Later in the thread I showed my equivalent https://www.avrfreaks.net/commen...

// ADC interrupt service routine
// with auto input scanning
interrupt [ADC_INT] void adc_isr(void)
{
register static unsigned char input_index=0;

// Read the AD conversion result
   adc_raw[input_index]=ADCW;
// Select next ADC input
   if (++input_index >= ADC_CHANNELS)
      {
      input_index=0;
      }

   ADMUX=(FIRST_ADC_INPUT|ADC_VREF_TYPE)+input_index;

// Start the AD conversion
   ADCSRA |= (1 << ADSC);
} 

Now, for your needs it will be a bit different as your channels are not contiguous.  In that situation, I use an aux table of the channels.  The suggested "round robin lee" should uncover examples. 

 

[edit] https://www.avrfreaks.net/commen... has an example of auxiliary table of channel numbers.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Aug 7, 2018 - 07:42 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
Define "not working".  What is the refer3ence voltage?  What are the signal voltages >>right on the AVR pins<< ?  How many counts do you expect?  How many counts are you getting?

The code corresponds to a part of an LFO of an electric guitar effect. It uses an infinite loop in the main function and updates the values ​​(sound effect) of frequency, wave type and others when there are modifications in the potentiometers (I assume that this is why it uses ISR).

 

Originally, the code changed the wave type and the time divisions according to the received voltage values ​​ADC2 and ADC3, respectively, in this function. It uses other ISRs to update other parameters, but I understand that this function is what I need to work on for the following: now I want to add a third function using a free channel, which will send a voltage signal through ADC0 that is unused. The received voltage values ​​are between 0 and 5v for each channel.

 

To make the tests I have a device that turns on an LED according to the type of wave and the division chosen, with voltage variators. Although it is not "sound", with the way of turning the LED on and off it is enough to disign if it is a sine wave, a square wave, etc. In that sense, the original code works perfectly. However, when I modify this function to try to read a third channel, even when it enters that case "do nothing", the behavior is no longer correct.

 

theusch wrote:
Personally, I find repetitive coding isn't any fun, and is error prone.  Later in the thread I showed my equivalent https://www.avrfreaks.net/commen...
Based on the code you just mentioned, it would not be more or less something like this?

ADMUX = ADC_VREF_TYPE + input_channel; // where input_channel would be 0, 2 or 3 (ADC0, ADC2 and ADC3)

If so, it would be necessary to define 

#define ADC_VREF_TYPE 0x40;

And it should be modified at the beginning:

sample_value = ADCW;

And at the end of the switch:

ADCSRA |= (1 << ADSC);

Or am I wrong?

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

While you gave much effort giving background  information, you didn't really answer any of my questions.  There is a reason for each.

 

I twice told you about "losing" ADLAR, and thus you get only 2-bit results.  You have never addressed that.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
I twice told you about "losing" ADLAR, and thus you get only 2-bit results.  You have never addressed that
  Sorry, I omitted commenting that I updated the way to initialize ADMUX. Is that enough to solve what you mentioned about ADLAR?

ADMUX = (1 << MUX1) | (1 << MUX0);

 

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

elbraca wrote:
Is that enough to solve what you mentioned about ADLAR? ADMUX = (1 << MUX1) | (1 << MUX0);

???  No.  After that statement, what is the value of the ADLAR bit in ADMUX?

 

If you in fact start to answer my questions, including what "doesn't work" means, I'll continue to look at the thread and respond.  Otherwise, I'm out.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Your ADC0 is on PB5 which is the RESET pin on the Tiny85.  There is a fuse setting that turns off the RESET function for this IC but I DO NOT suggest that you change this fuse setting unless you have a real Atmel debugging hardware device like the Dragon.  Once you disable RESET you won't be able to program or even access the Tiny85 with any ISP-based tool, like the USBasp.

 

  Consider moving to the Tiny84, which has 14 pins instead of 8 pins.  These additional pins usually have ADC channels.  A 14-pin SOIC package is about the same physical size on a PCB as a DIP Tiny85 (8-pin).  They are about the same cost as well.

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

theusch wrote:
If you in fact start to answer my questions, including what "doesn't work" means, I'll continue to look at the thread and respond.  Otherwise, I'm out.
I'm sorry, I'm doing the best I can. I don't usually work with this type of systems. Now I will try to answer your questions promptly:

 

1) 

theusch wrote:
After that statement, what is the value of the ADLAR bit in ADMUX?
 The only place where a value was assigned to ADLAR was in the initialization: 

ADMUX = (1 << ADLAR) | (1 << MUX1) | (1 << MUX0);

It does not appear again in the rest of the code, for which I assume that if it is not initialized it will remain at its default value.

 

2) Doesn't work: means that they do not assign the wave type and the subdivision that it must have according to the voltage values received in ADC2 and ADC3, respectively. The voltage converter that powers ADC2 and ADC3 is moved but that change is not reflected in the LED response.

 

3)

theusch wrote:
and I hope it is not "free running"...
 It's free running.

 

4) 

theusch wrote:
What is the refer3ence voltage?
 Between 0 and 5v for each of the channels, depending on the position of the voltage variator.

 

5) 

theusch wrote:
What are the signal voltages >>right on the AVR pins<< ?
I assume you mean the following:

- ADC2 handles the wave type
- ADC3 handles tempo subdivisions

- ADC0 is free now

 

6) 

theusch wrote:
How many counts do you expect?  How many counts are you getting?
 How many counts do you expect? I don't know specifically here what you mean. If you refer to the number of times the LED is lit, that depends on the TAP tempo (which is another entry that is not included in this ISR) and the subdivisions of tempo (this is what this function does). In fact, the TAP tempo works correctly.

 

I think those are all. If I missed any, please let me know. Thanks for your help!

Last Edited: Tue. Aug 7, 2018 - 09:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Simonetta wrote:
Consider moving to the Tiny84, which has 14 pins instead of 8 pins.  These additional pins usually have ADC channels.  A 14-pin SOIC package is about the same physical size on a PCB as a DIP Tiny85 (8-pin).  They are about the same cost as well.
Thank you very much for the advice. I will look for information about the attiny84, to serve me for this application, I will take it into account if I can not solve the problem with the 85.

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

ADLAR is not a separate register, it is a bit within a register (ADMUX).  Every time you write to ADMUX you are writing all 8 bits within that register.  If you don't explicitly set a given bit position to '1' when you write that (or any) register, it will be written with a '0' by default.  Do you intend to set all the bits in ADMUX (except MUX0 and MUX1) to '0' every time you write to the register?

Last Edited: Tue. Aug 7, 2018 - 09:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

elbraca wrote:
for which I assume that if it is not initialized it will remain at its default value.

Stop right there. ???

 

If you have a variable xyz and you write xyz = 123; and then you write xyz = 103; what makes you think that the tens digit remains at 2?

 

The original code did bit-manipulation to change the channel, and kept the ADLAR value the same.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

kk6gm wrote:

ADLAR is not a separate register, it is a bit within a register (ADMUX).  Every time you write to ADMUX you are writing all 8 bits within that register.  If you don't explicitly set a given bit position to '1' when you write that (or any) register, it will be written with a '0' by default.  Do you intend to set all the bits in ADMUX (except MUX0 and MUX1) to '0' every time you write to the register?

 I think here could be my first mistake. When I did, for example: 

// ...
// Go from ADC2 to ADC3
ADMUX = (1 << MUX1) | (1 << MUX0);
// ...

I thought I was just modifying those two bits of ADMUX and not the entire registry. In fact, every time I try to "jump" from one channel to the other (either from ADC0 to ADC2, ADC2 to ADC3 or ADC3 to ADC0) the only thing I did was to modify MUX and MUX. From what you say, then I should assign values to the entire register to "fix" the jumps from one channel to another, right?

 

theusch wrote:
If you have a variable xyz and you write xyz = 123; and then you write xyz = 103; what makes you think that the tens digit remains at 2?
I just think that there was my mistake, as I mentioned above. I thought that only two bits were modified and not the entire record.

 

theusch wrote:
The original code did bit-manipulation to change the channel, and kept the ADLAR value the same.
Exactly, the intention is to use the same mechanics, since it seemed simple. The question now is: how do I adapt it to use the 3 channels?

Last Edited: Tue. Aug 7, 2018 - 10:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

elbraca wrote:
Exactly, the intention is to use the same mechanics, since it seemed simple. The question now is: how do I adapt it to use the 3 channels?

Why don't you just rebuild ADMUX each time, with the desired reference selection and other options?  Yes, some seem to prefer manipulations with masking but why not just build the desired value?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:

Why don't you just rebuild ADMUX each time, with the desired reference selection and other options?  Yes, some seem to prefer manipulations with masking but why not just build the desired value?

I agree.  If you know at all times what the other bits will be, just write out channel constants that incorporate those bits.  ORing and ANDing is only required when some other portion of code may have changed other bits.

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

theusch wrote:
Why don't you just rebuild ADMUX each time, with the desired reference selection and other options?  Yes, some seem to prefer manipulations with masking but why not just build the desired value?
Do you think something like that could work? For example, suppose we are in ADC0 and we want to read ADC2 in the next one:

// ..
case (0x00):   // ADC0 MUX[1:0] == 00
  // Do something here
  // Update ADMUX to ADC2
  ADMUX |= (0 << REFS1) | (0 << REFS0) | (0 << ADLAR) | (0 << REFS2) | (0 << MUX3) | (0 << MUX2) | (1 << MUX1) | (0 << MUX0);
  break;
// ...

I'm not sure if it should be assigned with | = or directly with =.

Last Edited: Wed. Aug 8, 2018 - 12:17 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You're working too hard (and you're still setting ADLAR to 0!).

 

It's enough to write

ADMUX = (1<<ADLAR) | 3; //0, 2 or 3

This works because the MUX bits are the 4 LSBs of the register.  More generally you could write

ADMUX = (1<<ADLAR) | (3<<MUX0); //0, 2 or 3

 

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

kk6gm wrote:

You're working too hard (and you're still setting ADLAR to 0!).

 

It's enough to write

ADMUX = (1<<ADLAR) | 3; //0, 2 or 3

All right! I made some modifications. It was as follows:

ISR(ADC_vect)
{
  static bool first_sampling = true;
  uint8_t sample_value;
  sample_value = ADCH;
  
  if (first_sampling)
  {
    first_sampling = false;
  }
  else
  {
    switch (ADMUX & 0x0F)
    {
      case (0x00):   // ADC0 MUX[1:0] == 00
        // Do nothing here...
        // Update ADMUX to ADC2
        ADMUX = (1 << ADLAR) | 2;
        break;
      case (0x02):  // ADC2 MUX[1:0] == 10
        // SubdivisionFunction();
        // Update ADMUX to ADC3
        ADMUX = (1 << ADLAR) | 3;
        break;
      case (0x03):  // ADC3 MUX[1:0] == 11
        // WaveFormFunction();
        // Update ADMUX to ADC0
        ADMUX = (1 << ADLAR) | 0;
        break;
      default:
        break;
    }
  }
}

It improved, but it still doesn't work as it should when the voltage values of ADC2 and ADC3 are modified. I tried removing the first part of the switch (case 0x00), so it should respond as it did originally, but it did not work either. What could we be overlooking?

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

Show the smallest, complete, buildable program which demonstrates the problem.

 

Show your schematic.

 

Explain what is connected to ADC0, ADC2, and ADC3.

 

I echo Simonetta's question about ADC0/RESET.

"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."

"Read a lot.  Write a lot."

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

 

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

I thought I was just modifying those two bits of ADMUX and not the entire registry

Why on earth would you think that?

 

You are saying register ADMUX=something:   ADMUX = (1 << MUX1) | (1 << MUX0);
 

Which of course is exactly the same as ADMUX = (0<<ADLAR) | (1 << MUX1) | (1 << MUX0);
I always include ALL 8 bits in the assignment for clarity, just to avoid such "misunderstandings":

 ADMUX = (0<<REFS2) | (0<<REFS1) |  (0<<REFS0) |  (1<<ADLAR) |  (0 << MUX3) | (0 << MUX2) | (1 << MUX1) | (1 << MUX0);

 

There, no ambiguity as to what is being configured, it forces you to choose each!

 

want to flip two of then bits, pick them:

ADMUX ^= (0<<REFS2) | (0<<REFS1) |  (1<<REFS0) |  (1<<ADLAR) |  (0 << MUX3) | (0 << MUX2) | ( 0<< MUX1) | (0 << MUX0);  //REFSO & ADLAR WILL GET FLIPPED

 

Once you get your feet wet, you might consider deleting the  "zero" terms, but I still find them useful when I'm hunting for bits to manage...where is the damn !#??#!!#!! bit.

When in the dark remember-the future looks brighter than ever.

Last Edited: Wed. Aug 8, 2018 - 04:04 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I usually do something like:

#define ADMUX_SETUP   ((0 << REFS2) | (0 << REFS1) | (0 << REFS0) | (1 << ADLAR)) // Set reference voltage and ADLAR as required

// And then in the code...

ADMUX = ADMUX_SETUP | (set_mux);

// where set_mux is the particular ADC I want to access

YMMV

David (aka frog_jr)

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

Sorry for the delay in responding. Thank you very much everyone for the help!

 

I was doing some tests, but first I would like to mention some considerations. In relation to what was mentioned by Simonetta and joeymorin abot ADC0, it seemed to me that the most important thing would be to first ensure that ADC2 and ADC3 work correctly, as they did originally. So for a moment I "discarded" the inclusion of ADC0, until the original and the modified code work the same.

 

For that, I took the original code and rewrote it with all the suggestions proposed here (forgetting that I really want to include ADC0). If we do not mess up, they should work the same way.

Original code:

int main()
{
  // ...
  ADMUX = (1 << ADLAR) | (1 << MUX1) | (1 << MUX0);
  // ...
  
  while (true)
  {
  }
}


ISR(ADC_vect)
{
  static bool_t first_sampling = false;
  uint8_t sample_value;
  sample_value = ADCH;

  if (first_sampling == true)
  {
    first_sampling = false;
  }
  else
  {
    if (ADMUX & 0x01)   // MUX[0:0] == 1
    {
      SetWaveform(sample_value);
    }
    else    // MUX[0:0] == 0
    {
      SetMultiplier(sample_value);
    }
    // Switch the ADC input channel
    ADMUX ^= (1 << MUX0);   // toggle between MUX[1:0] = 11 and MUX[1:0] = 10
    first_sampling = true;
  }
}

New code A:

#define ADMUX_SETUP ((0 << REFS2) | (0 << REFS1) | (0 << REFS0) | (1 << ADLAR))
// ...

int main()
{
  //...
  ADMUX = ADMUX_SETUP | 2;
  //...
  
  while(true)
  {
  }
}

ISR(ADC_vect)
{
  static bool first_sampling = true;
  uint8_t sample_value;
  sample_value = ADCH;
    
  if (first_sampling)
  {
    first_sampling = false;
  }
  else
  {
    switch (ADMUX & 0x0F)
    {
      case (0x02):  // ADC2 MUX[1:0] == 10
        SetMultiplier(sample_value);
        // Update ADMUX to ADC3
        ADMUX = ADMUX_SETUP | 3;
        break;
      case (0x03):  // ADC3 MUX[1:0] == 11
        SetWaveform(sample_value);
        // Update ADMUX to ADC0
        ADMUX = ADMUX_SETUP | 2;
        break;
      default:
        break;
    }
  }
}

On impacting both codes, I noticed the following: in the new code, when modifying the voltage values of ADC2 and ADC3 they were "inverted", that is, when modifying ADC2 it behaved as if modifying ADC3 and vice versa. Therefore, it occurred to me to make the following modification and prove what happened:

New code B:

// the rest equal to the new code A above...

ISR(ADC_vect)
{
  // the rest equal to the new code A above...

    switch (ADMUX & 0x0F)
    {
      case (0x02):  // ADC2 MUX[1:0] == 10
        SetWaveform(sample_value);
        // Update ADMUX to ADC3
        ADMUX = ADMUX_SETUP | 3;
        break;
      case (0x03):  // ADC3 MUX[1:0] == 11
        SetMultiplier(sample_value);
        // Update ADMUX to ADC0
        ADMUX = ADMUX_SETUP | 2;
        break;
      default:
        break;
    }
  }
}

That is, I inverted the places where the functions that handle the wave type and the subdivision are called. Therefore, it was to be expected that the new A code will now work "upside down" by modifying the voltage values in ADC2 or ADC3 (and, therefore, the same as the original code). However, the new A and B codes work exactly the same. In other words, for the new code it does not seem to matter where in the two available I call the multiplier and wave type functions (and that's definitely not right).

 

From this I could determine that there is a problem with the new way of writing the ISR function (ADC_vect). I could not distinguish which one it is, but it seems to me that the most sensible thing is to be able to correct this first and then turn to add the ADC0 ...

 

If you think it is useful to include all the main function code, I can put it. I omitted it so that it would not be too long. Soon I will upload an schematic to make it easier to follow.  Meanwhile, I think the following summary will be useful:

- ADC2 = PB4: Regulates the tempo subdivisions (SetMultiplier Function) -> 0 to 5v
- ADC3 = PB3: Regulate the wave type (SetWaveForm Function) -> 0 to 5v

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

You seem to have ignored me:

joeymorin wrote:
Show the smallest, complete, buildable program which demonstrates the problem.

 

 

elbraca wrote:

when modifying ADC2 it behaved as if modifying ADC3 and vice versa.

You're using free running mode, right?  That's expected.  When a conversion completes, a new conversion is started and the ISR fires.  The ISR examines ADMUX to determine what the current channel is, then sets ADMUX to the next channel.  The trouble is, the ADC is already undertaking the next conversion but with the old value of ADMUX, so the next ISR will receive the sample for the last channel, even though you examine ADMUX and find that corresponds to the next channel.

 

Moral of the story:  be careful with free running when round-robining over multiple channels.  The ADC results are always one channel behind.

"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."

"Read a lot.  Write a lot."

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

 

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

joeymorin wrote:
Show the smallest, complete, buildable program which demonstrates the problem.
In relation to this, I have the following question: the tests I do are not made on software, but on hardware directly. The attiny85 is burned using SketchMonkey and a circuit built specifically for that in order to be able to use 12v. Having said that, it is useful to place all the code (practically all functions intervene in the final result, since there is a selection of wave type, tap tempo, sub-divisions of tempo, and so on) or it is useful that you place them defined, the main and the functions that exclusively intervene in this ISR?

 

joeymorin wrote:
You're using free running mode, right?  That's expected.  When a conversion completes, a new conversion is started and the ISR fires.  The ISR examines ADMUX to determine what the current channel is, then sets ADMUX to the next channel.  The trouble is, the ADC is already undertaking the next conversion but with the old value of ADMUX, so the next ISR will receive the sample for the last channel, even though you examine ADMUX and find that corresponds to the next channel.   Moral of the story:  be careful with free running when round-robining over multiple channels.  The ADC results are always one channel behind.
Yes, it is free running and I have no other choice since it is an LFO for an electric guitar effect.

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

I think this has already been said, but PB5(ADC0) is also the reset pin, if the fuse to disable the reset function has not been set, then there is an internal pullup and thus analog readings will have a bias to them and read higher for a given signal level, and if the signal level drops below the reset threshold then the chip will "Reset".

If the fuse is set then programming the chip gets very difficult!

 

Jim

 

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

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

elbraca wrote:

...I have no other choice since it is an LFO for an electric guitar effect.

That is pure and utter nonsense.  Please give me any justification for this.  Include the sample rates needed; your AVR's clock speed. Tell how you are going to handle programming your AVR once you have no reset pin.

 

Free-running, and "don't do that", have been said earlier.  You apparently choose to ignore that. 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
Free-running, and "don't do that", have been said earlier.  You apparently choose to ignore that.
I wouldn't say don't do it, just that you have to be aware of the pitfalls, namely that the changes you make to ADMUX now will apply not to the next result you see from the ADC, but to the one after that, which must be accounted for. I use free-running frequently, and it posed no other challenges w.r.t. software. Mind you, the source impedance is a greater issue for free-running mode because the S/H cap gate is on for only 1.5 ADC clock cycles.
EDIT: sp

"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."

"Read a lot.  Write a lot."

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

 

Last Edited: Fri. Aug 10, 2018 - 07:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As joeymorin pointed out, the ADC is using the "wrong" value of the MUX due to the auto-trigger. So instead of using the auto-trigger, you can initiate a new conversion immediately after changing the MUX bits.  This will ensure you know which input was sampled next time through the ISR:

ISR(ADC_vect)
{
  //...
  {
    switch (ADMUX & 0x0F)
    {
      case (0x02):  // ADC2 MUX[1:0] == 10
        SetMultiplier(sample_value);
        // Update ADMUX to ADC3
        ADMUX = ADMUX_SETUP | 3;
        break;
      case (0x03):  // ADC3 MUX[1:0] == 11
        SetWaveform(sample_value);
        // Update ADMUX to ADC0
        ADMUX = ADMUX_SETUP | 2;
        break;
      default:
        break;
    }
    ADCSRA |= (1<<ADSC);  // add this and don't use ADATE
  }
}

I would also put some code in the default case to set the MUX to a good value even though it should never get there.

 

--Mike

 

EDIT: fixed typo

 

Last Edited: Sat. Aug 11, 2018 - 04:37 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Mind you, the source impedance is a greater issue for free-running mode because the S/H cap gate is on for only 1.5 ADC clock cycles.

OK, I'll bite -- how is that any different than individual ADSC, or any other auto-trigger source?

 

I'm glad you "enjoy" free-running.  Especially in multi-channel work, I've [almost] always chosen to pick the channel and start the conversion.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
OK, I'll bite -- how is that any different than individual ADSC, or any other auto-trigger source?
Many a thread on this, and yes, the data sheets are not very revealing, but the gate is always on i.e. the selected MUX is always connected to the S/H cap >>except<< while a conversion is underway. For free-running mode, this amounts to the first 1.5 ADC clock cycles. For other auto-triggered modes, the gate will remain on for whatever the delta is between the end of the last conversion and the new trigger plus 1.5 ADC clock cycles.
Not near a computer, so can't easily search for threads.
Free-running gets you 'fast as possible' sample rates, and consistency. Fetch and set ADC does not (easily). Granted neither is always important.

"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."

"Read a lot.  Write a lot."

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

 

Last Edited: Fri. Aug 10, 2018 - 07:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Many a thread on this, and yes, the data sheets are not very revealing,

Interesting.  I recall no threads, nor datasheet info.  It doesn't make much sense to me on a couple fronts -- the ADC "knows", out of all the auto-trigger sources as well as manual, that it is source 0 and takes special action?  Why would that be?  What impetus would the chip designers have to make this exception to operation?

 

The other puzzlement is, if the "gate is open except when a conversion is in progress", and auto-trigger >>alwyas<< has a conversion in process, how is the gate open?  And what about the first 1.5 ADC clock cycles after a conversion is started?  The gate closes?  Then S/H happens with the gate closed?

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Not an exhaustive list:

https://www.avrfreaks.net/comment/1447326#comment-1447326
https://www.avrfreaks.net/comment/1308096#comment-1308096
https://www.avrfreaks.net/comment/1722446#comment-1722446
https://www.avrfreaks.net/comment/2410816#comment-2410816
https://www.avrfreaks.net/comment/924195#comment-924195
https://www.avrfreaks.net/forum/how-long-does-sample-and-hold-take

 

It doesn't make much sense to me on a couple fronts -- the ADC "knows", out of all the auto-trigger sources as well as manual, that it is source 0 and takes special action?

I'm sorry, you've lost me/  What do you mean by 'source 0'?

 

This is a departure from the OP.  His issue is that by using free-running mode, each sample he is putting to use is actually one sample in the past.  With his round-robin approach, what he thinks is a sample from channel 3 is therefore in fact a sample from channel 2, and vice versa.

 

 

"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."

"Read a lot.  Write a lot."

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

 

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

joeymorin wrote:
What do you mean by 'source 0'?

Auto-trigger on most [somewhat recent] AVR8 models has a table

• Bit 2:0 – ADTS2:0: ADC Auto Trigger Source

of auto-trigger sources, and free-running is [always?] source 000.

 

I'll run through the links to refresh my memory.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
I'll run through the links to refresh my memory.

The first link (the most recent in date?) basically re-affirms what you say here.

 

The second link in your list is quite interesting, where you say:

joeymorin wrote:

 

That's an intriguing claim.

I see no mention of this kind of behaviour in any of the datasheets I've read. Further, the bench tests I've done suggest this to be untrue.

So long as the ADC is enabled the S/H cap is connected to whichever input is selected by ADMUX. It will follow the voltage on that input, ...


Isn't that what I'm saying here?

 

On to the other links, where presumably you found the free-running thing?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
On to the other links, where presumably you found the free-running thing?

So maybe it is the third link

Changing the MUX disconnects the cap from the previous channel and re-routes the signal from the new channel to the cap.  If the MUX is changed immediately preceding the start of a new conversion, the cap will have only 1.5 ADC clock cycles to charge to the new channel's voltage.  If the voltage differential between the previous and new channels is high, and if the new channel's source impedance is high, the cap may not have enough time to settle to the new channels' voltage before the 'hold'.

Is that what you are referring to?  Yes, "channel bleedthrough" is real and IMO/IME has been proven.  So, how is free-running any different?  Yes, there may be a marginal time change longer with a single-conversion setup with mux change and ADSC in the 'complete' ISR.  Say a couple microseconds, or about another ADC clock?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Isn't that what I'm saying here?

Keep reading that thread.

 

So, how is free-running any different?

I'm not clear on what you're objecting to.  Perhaps I was answering the wrong question with the links I posted?

 

Here's what seems to have started this diversion:

 

theusch wrote:
joeymorin wrote:

Mind you, the source impedance is a greater issue for free-running mode because the S/H cap gate is on for only 1.5 ADC clock cycles.

OK, I'll bite -- how is that any different than individual ADSC, or any other auto-trigger source?

 

Free-running leaves the gate on for exactly 1.5 ADC clock cycles, no more, no less.  Whereas other auto-trigger modes will leave it on for at least 2 cycles, but the actual time will be the delta of the actual sample period minus the conversion time.  This can be very much longer than 2 cycles.

 

Manual retriggering via ADSC cannot have a sample rate as fast as free-running, without careful cpu cycle counting or other synchronisation to ensure that ADSC is again set on the very cycle that the current conversion completes.  Generally, you'll find that the best you do is 13.5 ADC clock cycles or longer (just like the other auto-trigger modes)

 

The datasheets are not clear on the a number of things.  In particular, the timing diagrams have 'MUX and REFS update' and 'Sample & Hold'.

 

The second of those is particularly unclear.  'Sample' is not the same as 'hold' and they don't happen at the same time.  In any ADC, first the S/H cap samples the input.  This is not a moment in time, rather a time interval.  The 'hold' is the moment when the sampling ends, the gate is switched off, and the S/H cap is isolated from the input.  The point in the datasheet timing diagrams marked 'Sample & Hold' is actually just the 'hold'.  The 'sample' is the interval preceeding it.

 

The first of those, 'MUX and REFS update' is misleading as well because, at least to me, it suggests that the hardware will switch MUX/REFS at the moment.  In reality, that point in the timing diagram is where the MUX/REFS bits are latched.  Changes to those bits after the latching, and before the end of the associated conversion, are buffered in the same way that changes to OCRnx are buffered for PWM modes.  They are not ignored, but the effect is deferred.  Changes made to those bits in the interval between the end of a conversion and the point at which they are latched at the beginning of the next conversion are immediate.

 

In free-running mode, that interval is zero.

 

My point in #36 was tangential to the OP's issue but boils down to the observation that with longer sample sample intervals, the S/H cap has more time to charge to the input voltage before the 'hold' and conversion, and the higher the source impedance can be while still allowing accurate conversions.  Free-running exhibits the shortest sample interval of all the modes available, 1.5 ADC clock cycles, so impedance is a tighter constraint.

 

The OP's issue, however, is still the same:  each change he makes to ADMUX in the ISR will affect not the next conversion, but the conversion >>after<< it.

"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."

"Read a lot.  Write a lot."

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

 

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

joeymorin wrote:
Keep reading that thread.

I did.

joeymorin wrote:
The OP's issue, however, is still the same: each change he makes to ADMUX in the ISR will affect not the next conversion, but the conversion >>after<< it.

Indeed, so since AVR8 are essentially obsolete and the ADC characteristics are not going to change, there were a number of points made for the OP.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.