Actions
Forum Thread Details
  • Replies 26 replies
  • Subscribers 384 subscribers
  • Views 4280 views
  • Users 0 members are here
Related

Rate my arduino code

scottiebabe

I have never used an actual arduino board and have only tried the arduino framework on non-arduino hardware a few times.

I am putting together a little test box for an ADS1115 ADC module (or clone of  New clone on the block  ) 

Quite stuff inside the box, noisy stuff outside the diecast box, nothing fancy.

image

I am using a RPI Pico running the arduino framework using platformio

image

Here is the code I put together using Adafruits arduino library for the ADS1115

// Sample an ADS1115 ADC via i2c with a RPI Pico
// PICO GPIO20 - I2C_SDA
// PICO GPIO21 - I2C_SCL
// PICO GPIO16 - ADS1115_DRDY
//
// Inspired by Adafruit demo, use as you like.
// scottiebabe/sstobbe@2n3904blog.com

#include <Wire.h>
#include <Adafruit_ADS1X15.h>

// GPIO16 ISR
void ADSInterrupt();

Adafruit_ADS1115 ads1115;
MbedI2C i2c(20,21);  // Use GPIO Pins 20,21 on Pico for i2c

void setup(void)
{
  Serial.begin(9600);
  //while (!Serial);
  delay(1000);
  Serial.println("Hello!");
  Serial.println("Getting single-ended readings from AIN0..3");
  Serial.println("ADC Range: +/- 6.144V (1 bit = 187.5uV)");
  ads1115.begin(0x48,&i2c);

  // Enable Pico's onboard LED
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN,1);

  // Setup ISR on GPIO16
  pinMode(16,INPUT_PULLUP);
  attachInterrupt(16, ADSInterrupt, FALLING );
  gpio_pull_up(16);

  Serial.println("Hello, pt2!");
  Serial.println("Configured ADS1115 for single channel continuous conversion.");
  Serial.println("ADC Range: +/- 256mV (1 bit = 7.8125 uV)");
  Serial.println("Fs: 860 Sa/s");
  Serial.println("Ain: Ain0-single ended");
  //ads1115.setGain(GAIN_TWOTHIRDS); // SET FSR +-6V
  ads1115.setGain(GAIN_SIXTEEN); // Set FSR +- 256 mV
  ads1115.setDataRate(RATE_ADS1115_860SPS);
  //ads1115.startADCReading(ADS1X15_REG_CONFIG_MUX_DIFF_0_1,true);
  Serial.println("Starting Acquisition! ...");
  ads1115.startADCReading(ADS1X15_REG_CONFIG_MUX_SINGLE_0,true);
}

#define ADS_BUF_LEN (4096)
volatile int16_t  samples[ADS_BUF_LEN] = {0};
volatile uint32_t tstamps[ADS_BUF_LEN] = {0};
volatile int adsidx = 0;
volatile int adsinterrupt = 0;

// GPIO16 ISR
void ADSInterrupt() {
  if( adsidx < ADS_BUF_LEN ){
    tstamps[adsidx] = time_us_32();
  }
  adsinterrupt = 1;
}

/* Adafruit Demo Loop
void loop(void)
{
  int16_t adc0, adc1, adc2, adc3;

  adc0 = ads1115.readADC_SingleEnded(0);
  adc1 = ads1115.readADC_SingleEnded(1);
  adc2 = ads1115.readADC_SingleEnded(2);
  adc3 = ads1115.readADC_SingleEnded(3);
  Serial.print("AIN0: "); Serial.println(adc0);
  Serial.print("AIN1: "); Serial.println(adc1);
  Serial.print("AIN2: "); Serial.println(adc2);
  Serial.print("AIN3: "); Serial.println(adc3);
  Serial.println(" ");
  
  delay(1000);
}
*/

void loop(void){

  if(adsinterrupt){
    if( adsidx < ADS_BUF_LEN ){
      samples[adsidx] = ads1115.getLastConversionResults();
      adsidx++;
    }
    adsinterrupt = 0;
  }

  if(adsidx >= ADS_BUF_LEN){
    //Print out the sample buffer to serial console
    Serial.println("Acquisition Buffer:");
    for(int i=0; i<ADS_BUF_LEN; i++){
      Serial.print(tstamps[i]);
      Serial.print(",");
      Serial.println(ads1115.computeVolts(samples[i]),7);
      delay(1);
    }

    
    // dummy read to re-arm interrupt
    ads1115.getLastConversionResults();
    adsidx = 0;
    adsinterrupt = 0;
    Serial.println("Starting Acquisition! ...");
  }
}

Thumbsup / Thumbsdown

  • in reply to scottiebabe
    scottiebabe said:
    using the Arduino framework I'm not sure that matters since there is so much additional logic running before my ISR.

    There isn't. Arduino doesn't have much runtime overhead. It's a bit depending on who wrote the port for a controller family.

  • in reply to Jan Cumps

    Using this decorator puts my function in sram

    image

    image

  • It is always very subjective to rate someone's code.  Some of the comments here are good ideas.  I generally find your code to be clear and readable.  I don't see any overly complex or confusing logic, which is always sign of good coding.

  • in reply to genebren

    I do it as part of my job. One step is to recognise that you are subjective, and correct yourself.

  • The Pico SCPI instrument that we built here on the forum, uses the same ADC without the Arduino library / layer.
    We don't use interrupts. In the common SCPI query / response mechanism, we can get away without it. This would be different if the device was built for streaming / acquisition.

    Code: https://github.com/jancumps/pico_scpi_usbtmc_labtool/blob/main/source/adc16/adc16_utils.c

  • Code looks reasonable.

    I prefer more comments about the purpose of the code and maybe a revision list.

  • in reply to genebren

    Thanks for the kind words. I was pretty impressed how easy it up and running. I pretty much just copy pasted adafruit's demo code as a starting point....

  • in reply to Jan Cumps

    I totally understand why this ADC module is so popular! If google ADS1115 + (arduino or rpi) you'll find hundreds of projects and sample code for pretty much every platform in every language. For the no-name price its a no-brainer, its worth keeping a few around, even the genuine EVB is worth it for a one time purchase.   

    Because the input is un-buffered you can sample -200 mV single-ended. Of course you can sample negative inputs differentially too. Perhaps just use the native signed 16-bit value?

    Here is a cardiac signal action shot at least -20 dBFS (+- 256 mV range)

    image

  •   As a confirmed C programmer and just getting into JAVA(I like it as the syntax is almost the same) I and most of the folks I know always put globals, #define, structs, and whatnot in my headers or file.h  either main.h or file.h and they are always in a subdir called inc or include.  I will now rub more folks the wrong way by saying I never use the void loop{} construction as you can't return var, error codes, and things like that to the above program and you can't have a program with command line args, so I always use the standard invocation of int main( argv, argc) {}... 
    BTW in your void setup(){} my code is normally this:

    void setup(){
    #include "inc/setup.h"
    }

    

    The reason for not using the void loop(){} construction it does not provide me with a return value! In my sim and the most thing I write before I do anything I always test my results, so lets say you have two cpus closely coupled by some sort of communication link. ( the following is agnostic.) CPU1 expects a start sequence from CPU2, so what happens when CPU2 is late CPU 1 loops for 10~ times, if it never sees the message, CPU1 com fails with an error code stating that CPU2 is mute, then the program can exit cleanly with a error, maybe even into a log oh my so now you are back to test case maybe loop back, and the beat goes on.  
    

    Also please stop using the really cruddy ArduinoIDE as It is buggy and for system work it does not work well grab a copy of Ecipse C/C++ and If you must get the Arduino add-on and you get with this the GCC cross compiler. which really crosses to almost all compilers, and this way you have a constant IDE that is globally accepted.  ~~CAH
    
			
    

		
		
		
		
	
    
	
    
		
    
			
		
    
	
    
	
    
		
						
				
				
						
				
				
		
		
    
			
		
    
	
    

    

    • 

	

  • 
	
    








    
	
    
		
    
							
					
    
		
    
			
											
			
				
									
							
						
				
			
												
						in reply to DAB
					
									
    
	
    

		
		
    
					
      comments are always good, but not the PASCAL way where every line is a freaking comment.  A good comment block, Now on to the dirty word revisions!! THIS IS A MUST!!! most of the time when I use Eclipse my code sits in a local repo, or Git, whatever. My repo uses the "MAVIN" file layout. And I will get around to start using "Jenkins" which is a software automation tool. ~~CAH
    
			
    

		
		
		
		
	
    
	
    
		
    
			
		
    
	
    
	
    
		
						
				
				
						
				
				
		
		
    
			
		
    
	
    

    

    •