Hello all,
I know this is not really a question, and I may be posting this in the wrong place, but I am working on a project where the supply voltage will be on and off and the STM32 will turn on-and-off, so I wanted to minimize the execution speed by condensing the code. Is there someone who can condense this code? I am not worried about the functions though. Thanks for the help!
#include <EEPROM.h> //Import the EEPROM library for using the EEPROM commands
//-------------- Variable Definitions --------------
float volts; //Create variable to store voltage value
int ADCValue; //Create variable to store the ADC value (raw data coming in)
int firstReading = 100; //Create variable to store whether the MCU just turned on or if it is a subsequent value
int address; //Create variable to store current address
byte data; //Create variable to act as a buffer for the written EEPROM value
bool lock = 0; //Create variable to make sure that the read process does not run twice
//-------------- Function Definitions --------------
int FindNextAddress(int startAddress) {
for(int i=startAddress; i<EEPROM.length(); i++) { //Start stepping through each EEPROM address
if(EEPROM.read(i) == 0) { //If the byte has not been written
address = i;
return address; //Return the address
}
}
}
float ADC_to_Volts(int adc_val, float vbat, int ref) {
float results = adc_val * (vbat / ref); //Formula for ADC value to voltage
return results; //Return the calculated voltage
}
//-------------- Main Code --------------
void setup() {
Serial.begin(9600); //Begin the USART connection at 9600 baud
pinMode(A0, INPUT); //Declare the ADC pin as an input
pinMode(2, INPUT); //Declare the jumper pin as an input
address = FindNextAddress(0); //Set the address to the next available address
}
void loop() {
//------ Initial Readings ------
ADCValue = analogRead(A0); //Read the raw ADC value
volts = ADC_to_Volts(ADCValue, 5.0, 1023); //Convert raw ADC value to voltage reading
data = firstReading + (volts * 10); //Assemble byte to be uploaded to EEPROM
//------ Logic for Write ------
if(digitalRead(2) == LOW) { //Check if jumper is unplugged
EEPROM.write(address, (int)data); //Write the data assembled from earlier
address++; //Go to the next address
firstReading = 0; //Declare that this is no longer the first reading and is a subsequent value
delay(500); //delay by 500 ms to make sure entire EEPROM is not written in a short time by using a delay
}
//------ Logic for Read ------
else { //If the jumper is plugged in
if(!lock) { //If the value is not locked
for(int i=0; i<EEPROM.length(); i++) { //Step through every address in the EEPROM
Serial.println(EEPROM.read(i)); //Print each value in the EEPROM
}
lock = 1; //Lock the read function so it dosen't read recursivly
}
}
}
Something you can do to speed up your program is to avoid the use of floating point variables and floating point operations.
In your code, you convert integer values (digitalRead) to float (volt) and then back to integer. You can save CPU cycles here. Also, each every time you call
float ADC_to_Volts(int adc_val, float vbat, int ref) {
float results = adc_val * (vbat / ref); //Formula for ADC value to voltage
return results; //Return the calculated voltage
}
you are forcing the evaluation of the same floating point division (5.0/1023). You can compute a fixed factor just once in the setup and use it here repeatedly.
What is your code supposed to do?
You have to review your code carefully and decide, for example, what firstReading is for, how you will prevent the process from writing beyond the EEPROM capacity, or how you will manage to unplug the jumper before the EEPROM is full, and finally, how come you need more speed and then include this in your code:
delay(500); //delay by 500 ms to make sure entire EEPROM is not written in a short time by using a delay
Thank you for the response. First question. This code is for a small (40x40x40mm) cube. The plan is to make use of Faraday's Law to generate electricity by shaking a magnet through a copper coil rapidly. The voltage genration will go through logic to only let the voltage through when it is suitable enough for powering an MCU. This should write the voltage generated by measuring it with an analog pin. Second question. firstReading is for indicating whether or not the byte of data is a subsequent value or not. This will let us know if the MCU turned on and off rapidly, or if it was able to stably stay on. Fourth question. Thank you for the catch, that could have ended badly. Would something like: if((digitalRead(2) == LOW) && (address<EEPROM.length())) instead of if((digitalRead(2) == LOW)) (line 47) work? Fifth question. Would we need to be careful about this once we fix line 47 (answer to 4th question)? Sixth question. I need speed so that there are actually values being printed to the EEPROM. I want enough time to measure, encode, and write all in a short pulse of power. Last question. The delay is in just in case the power to the MCU is stable, we will be able to record long times, so the entire EEPROM does not get written in a matter of seconds. Thanks for all the help, and sorry if my answers are not what you were looking for.
You can do better than that. If you make it a constant, it will be computed once during compilation, and never again.
const float ADC_TO_VOLTS = 5.0/1024;
Notice I used 1024 not 1023 which is not the correct scaling factor.
Or, since you have already made a complete function to do the conversion you could:
float ADC_to_Volts(int adc_val) {
float results = adc_val * 5.0/1024; //Formula for ADC value to voltage
return results; //Return the calculated voltage
}
Compiler optimization will ensure that the division is performed only at compile time, just as with the const variable.
I notice you convert the ADC to floating point and then back to integer. Isn't that inefficient? Why do you do it?
@ScienceMax
Check if the following version is a condensed form of your original sketch.
#include <EEPROM.h> //Import the EEPROM library for using the EEPROM commands
int firstReading = 100; //Create variable to store whether the MCU just turned on or if it is a subsequent value
int address; //Create variable to store current address
bool lock = 0; //Create variable to make sure that the read process does not run twice
void setup()
{
Serial.begin(9600); //Begin the USART connection at 9600 baud
pinMode(A0, INPUT); //Declare the ADC pin as an input
pinMode(2, INPUT_PULLUP); //Declare the jumper pin as an input
address = FindNextAddress(0); //Set the address to the next available address
}
void loop()
{
int ADCValue = analogRead(A0); //Read the raw ADC value
float volts = ADCValue * (5.0 / 1023); //32-bit binary32 format = 4-byte
float data = (float)firstReading + (volts * 10.0); //Assemble byte to be uploaded to EEPROM
if (digitalRead(2) == LOW)
{
EEPROM.put(address, data);
firstReading = 0;
}
else
{
if (!lock)
{
float y;
EEPROM.get(address, y);
Serial.println(y, 2);
lock = 1;
}
}
}
int FindNextAddress(int startAddress)
{
for (int i = startAddress; i < EEPROM.length(); i++)
{ //Start stepping through each EEPROM address
if (EEPROM.read(i) == 0)
{ //If the byte has not been written
address = i;
return address; //Return the address
}
}
}
youre doing an unnecessary conversion from into to float, which just wastes time, and gives a value in the range 0.0 to 4.995
then a float mult by 10, gives a value in the range 0.0 to 49.95
then losing all your 12 bit resolution by converting back to an INT.
I'd recommend you use int throughout, which you can do by working in millivolts.
ADCValue = analogRead(A0); //Read the raw ADC value
long int mVolts = (ADCValue * 50000) /1024; //including your *10 factor
..
data = firstReading + mVolts; //Assemble byte to be uploaded to EEPROM
One must test to determine efficiency. While code reviews do have some effect they are only thought experiments and can be wildly wrong.
I used to teach and had one class with a group of fun guys who delighted in showing how smart they were to the point of disrupting the flow of the lessons and making the other students feel inadequate.
One day the the lesson degenerated over statements that the lesson code was inefficient. I responded that perhaps we should have a lesson on efficiency which suggestion was well received.
The next day I brought in a PC and access to two different mainframes. I put simple code on the board with two ways of performing the same action and asked which was faster. I always told the responder that they were wrong before running the code. Then I selected which machine ran which version and yes those students were always wrong. When the other students started volunteering answers, told them they were right and they were.
Machines and compilers will do things differently and those differences can add up. Code reviews only go far.
One needs to test. Code profilers show you were your code consumes time and prevent spending days working on speed improvements to code that has little effect on total time.
For example in pseudo code
Array[900][900];
'For(i=1,900){
' For(j=1,900){
' Array[i][j];
' }
'}
Using i then j will usually be quite different from j then i in the brackets. Smart compilers may fix the issue. Change machines, software versions or compilers and the results may change. You need to test.