Variable Wont Change

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

I've got a bit of weird issue I can't figure out.

This doesn't work, BVALUE 3 never changes.

if (BVALUE[3]<9000) {BVALUE[3]+=1000;}
else {BVALUE[3] -= 9000;}
LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];		 

But this does work, BVALUE[2] changes

if (BVALUE[2]<9000) {BVALUE[2]+=1000;}
else {BVALUE[3] -= 9000;}
LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];		 

This also works,

				
if (BVALUE[3]<9000) {BVALUE[2]+=1000;}
else {BVALUE[3] -= 9000;}
LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[2]/1000)];		 

Basically this is part of a menu system which is done in a switch case statement. The previous cases are exactly the same as this one except they are BVALUE[0], BVALUE[1] and BVALUE[2], instead of BVALUE[3], they all work fine.

Is there anything stupidly obvious I have done wrong here that I am just not seeing? I can post the rest of the code if necessary, but it´s quite long and BVALUE is only used to perform a calculation in another function, so I don't want annoy anybody by posting a couple hundred lines of unnecessary code.
Here is the only other part of the code where BVAUE[3] is used,
Declared here,

volatile uint16_t BVALUE[5] = {3977, 3977, 3977, 3977};  //Beta value for NTC thermistors

and the calculation here,

for (uint8_t i=0; i<=3;i++)
{			
    fRntc = 10000 *  (float) ADCresult[i] / ( 1023.0  - (float) ADCresult[i]);

    fTemp = (1.0 / ( (log(fRntc/RREF))/BVALUE[i]  + 1.0/298.0)) - 273.0; 

   temperature[i] = fTemp * 10;

    if ((temperature[i] < 10) || (temperature[i] > 750)) 
    {
       errorCode = i+1;    //Save the error code
       TCCR2B |= 1<<CS21;  //Set the alarm
    }
}

I am using an ATMEGA328P and AVR Dragon debugger. I know this is going to be something stupid. Thanks.

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

It seems your code examples are not the actual code you have but rather something you just typed into the forum post. E.g. this seems to use a mix ob BVALUE[2] and BVALUE[2]:

if (BVALUE[2]<9000) {BVALUE[2]+=1000;}
else {BVALUE[3] -= 9000;}
LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];

Please post actual code you have, or the discussion will potentially be absolutely meaningless.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

No, that is the actual code I have copied and pasted from Atmel Studio 6. The first section of code that i said didn't work is the original code and the other examples are changes that I made to try and see what made a difference, to try and narrow the error down. So for example, when i change this line,

if (BVALUE[3]<9000) {BVALUE[3]+=1000;} 

to this,

if (BVALUE[3]<9000) {BVALUE[2]+=1000;} 

BVALUE[2] increments by 1000 in the second exmaple, which is correct, but the first example does nothing, BVALUE[3] never changes. This is what I am stuck on and what I don't understand. I don't know why it never changes.

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

What do the respective lss files say in that area for the two cases?

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

I´ll be honest, that I don´t have a clue what the ISS file does or what any of it means. I had a look and I think I have posted the relevant bits. The ISS is almost 8500 lines, so obviously don't want to post it all.

This is the original code

				 case 0:
					if (BVALUE[3]<9000) {BVALUE[3]+=1000;}
    2cd2:	ac e3       	ldi	r26, 0x3C	; 60
    2cd4:	b1 e0       	ldi	r27, 0x01	; 1
    2cd6:	8d 91       	ld	r24, X+
    2cd8:	9c 91       	ld	r25, X
    2cda:	11 97       	sbiw	r26, 0x01	; 1
    2cdc:	b3 e2       	ldi	r27, 0x23	; 35
    2cde:	88 32       	cpi	r24, 0x28	; 40
    2ce0:	9b 07       	cpc	r25, r27
    2ce2:	40 f4       	brcc	.+16     	; 0x2cf4 <_Z4menuv+0x27b6>
    2ce4:	fd 01       	movw	r30, r26
    2ce6:	80 81       	ld	r24, Z
    2ce8:	91 81       	ldd	r25, Z+1	; 0x01
    2cea:	88 51       	subi	r24, 0x18	; 24
    2cec:	9c 4f       	sbci	r25, 0xFC	; 252
    2cee:	91 83       	std	Z+1, r25	; 0x01
    2cf0:	80 83       	st	Z, r24
    2cf2:	0a c0       	rjmp	.+20     	; 0x2d08 <_Z4menuv+0x27ca>
					else {BVALUE[3] -= 9000;}
    2cf4:	ac e3       	ldi	r26, 0x3C	; 60
    2cf6:	b1 e0       	ldi	r27, 0x01	; 1
    2cf8:	8d 91       	ld	r24, X+
    2cfa:	9c 91       	ld	r25, X
    2cfc:	11 97       	sbiw	r26, 0x01	; 1
    2cfe:	88 52       	subi	r24, 0x28	; 40
    2d00:	93 42       	sbci	r25, 0x23	; 35
    2d02:	11 96       	adiw	r26, 0x01	; 1
    2d04:	9c 93       	st	X, r25
    2d06:	8e 93       	st	-X, r24

This is the modified test code

				 case 0:
					if (BVALUE[3]<9000) {BVALUE[2]+=1000;}
    2cd2:	ac e3       	ldi	r26, 0x3C	; 60
    2cd4:	b1 e0       	ldi	r27, 0x01	; 1
    2cd6:	8d 91       	ld	r24, X+
    2cd8:	9c 91       	ld	r25, X
    2cda:	11 97       	sbiw	r26, 0x01	; 1
    2cdc:	b3 e2       	ldi	r27, 0x23	; 35
    2cde:	88 32       	cpi	r24, 0x28	; 40
    2ce0:	9b 07       	cpc	r25, r27
    2ce2:	58 f4       	brcc	.+22     	; 0x2cfa <_Z4menuv+0x27bc>
    2ce4:	80 91 3a 01 	lds	r24, 0x013A
    2ce8:	90 91 3b 01 	lds	r25, 0x013B
    2cec:	88 51       	subi	r24, 0x18	; 24
    2cee:	9c 4f       	sbci	r25, 0xFC	; 252
    2cf0:	90 93 3b 01 	sts	0x013B, r25
    2cf4:	80 93 3a 01 	sts	0x013A, r24
    2cf8:	08 c0       	rjmp	.+16     	; 0x2d0a <_Z4menuv+0x27cc>
					else {BVALUE[3] -= 9000;}
    2cfa:	ec e3       	ldi	r30, 0x3C	; 60
    2cfc:	f1 e0       	ldi	r31, 0x01	; 1
    2cfe:	80 81       	ld	r24, Z
    2d00:	91 81       	ldd	r25, Z+1	; 0x01
    2d02:	88 52       	subi	r24, 0x28	; 40
    2d04:	93 42       	sbci	r25, 0x23	; 35
    2d06:	91 83       	std	Z+1, r25	; 0x01
    2d08:	80 83       	st	Z, r24

Like I said I have no idea what any of this means.
Thanks

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

Quote:

BVALUE[2] increments by 1000 in the second exmaple, which is correct, but the first example does nothing, BVALUE[3] never changes.

How are you determining this?

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

2 ways, firstly I have BVALUE[3] displayed on an LED display. Secondly I am using the debugger with a breakpoint set on the line after the if statement. With BVALUE[3] set, the variable doesn´t change and the display does not update. With it set as BVALUE[2] the variable does change, but the led doesn´t change, but that is correct, because the LED is set to display BVALUE[3].

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

I see how it is awkward (for both you and us) to post the complete code. But do post the complete functions where the above code snippets are present.

Is one of them an ISR, by any chance?

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

Ok, they are not ISR. The first function is what BVALUE is actually used for. I don´t think this has anything to do with it as this function is called from main() and main isn´t running at the point where I am trying to change BVALUE, I´ll explain that in a minute.

void getTemperature()
{
float fRntc;
float fTemp;

	ADCresult[ADCcurrentChanel] =  ADC;	//Save the value of the last ADC Compare
	switch (ADCcurrentChanel)
	{
		case 0:
		ADMUX = 0x41; //Clear the ADMUX Register to channel 1
		ADCcurrentChanel = 1;
		ADCSRA |= (1 << ADSC);			// Start A2D Conversions
		break;
		
		case 1:
		ADMUX = 0x42; //Clear the ADMUX Register to channel 2
		ADCcurrentChanel = 2;
		ADCSRA |= (1 << ADSC);			// Start A2D Conversions
		break;
		
		case 2:
		ADMUX = 0x43; //Clear the ADMUX Register to channel 3
		ADCcurrentChanel = 3;
		ADCSRA |= (1 << ADSC);			// Start A2D Conversions
		break;
		
		case 3:
		ADMUX = 0x40; //Clear the ADMUX Register to channel 4
		ADCcurrentChanel = 0;
		ADCSRA |= (1 << ADSC);			// Start A2D Conversions
		//All the temperature channels have been read so now we convert them to temperature
		for (uint8_t i=0; i<=3;i++)
		{
			// calculate NTC resistance , 1023 = 10 bits ADC  10000 = pullup value
			fRntc = 10000 *  (float) ADCresult[i] / ( 1023.0  - (float) ADCresult[i]);

			// calc temperature from NTC value
			fTemp = (1.0 / ( (log(fRntc/RREF))/BVALUE[i]  + 1.0/298.0)) - 273.0; //log = ln

			// multiply temperature by 10 so we can have 1 decimal place as an int (e.g. 25.3 = 253)
			temperature[i] = fTemp * 10;
			//Check for Sensor Errors
			if ((temperature[i] < 10) || (temperature[i] > 750)) //if the temperature is less the 1ºC or greater than 75ºC
			{
				errorCode = i+1;    //Save the error code
				TCCR2B |= 1<<CS21;  //Set the alarm
			}
		}
		temperatureToDisplay=true;  //set the temperature to display flag		
		break;
	}
}

The other function i will edit a bit just keep the page more readable. The cases that I have edited out adjust other values not related to BVALUE at all.

void menu()
{
	
uint8_t lastSeconds;
uint8_t selectedDigit = 0;
bool digitOnOff = 1;
uint16_t menuBuffer=0; //A buffer value used for adjusting settings
	
 while(menuMode)
 {
	switch (menuMode)
	{
////////////////////////////////TIME - Displays RTC////////////////////////////////////////////////////////////		
	case 1: 
	
	break;
//////////////////////////////Edit the time//////////////////////////////////////////////////////////////////////		
	case 2: 
							
	break;
///////////////////////////////Daytime Set point - Displays SPD///////////////////////////////////////////////////////////////////		
	case 3: 

	break;
///////////////////////////////Edit the Daytime Setpoint////////////////////////////////////////////////////////////////////////
	case 4:
		
	break;
///////////////////////////////Night time Set point - Displays SPN///////////////////////////////////////////////////////////////////		
	case 5: 
					
	break;
///////////////////////////////Edit the Night time Setpoint////////////////////////////////////////////////////////////////////////
	case 6:
															
	break;
////////////////////////////////LIGHTS ON TIME - Displays ONLI////////////////////////////////////////////////////////////
	case 7:
		
	break;
//////////////////////////////Edit the ON time//////////////////////////////////////////////////////////////////////		
	case 8: 
						
	break;
////////////////////////////////LIGHTS OFF TIME - Displays OFLI////////////////////////////////////////////////////////////
	case 9:
		
	break;
//////////////////////////////Edit the OFF time//////////////////////////////////////////////////////////////////////		
	case 10: 
						
	break;
///////////////////////////////NTC0 BETA VALUE - Displays BET1///////////////////////////////////////////////////////////////////		
	case 11: 
			LED_WhatToDisplay[0] = LED_Display_Char[11];
			LED_WhatToDisplay[1] = LED_Display_Char[14];
			LED_WhatToDisplay[2] = LED_Display_Char[23];
			LED_WhatToDisplay[3] = LED_Display_Char[1];
			//if the Select button is pressed
			if (!(SW_select_State))
			{
				SW_select_State=1;
				menuMode=12;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[0]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/1000)*1000))/100)];  
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[0]-((BVALUE[0]/10)*10)];
				selectedDigit=0;  
			}
			if (!(SW_Change_State)){SW_Change_State=1;menuMode=13;}		
	break;
///////////////////////////////ADJUST NTC0 BETA VALUE//////////////////////////////////////////////////////////////////////////////////			
	case 12:
			if ((digitOnOff)&&(selectedDigit==0)){LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[0]/1000)];}
			if ((digitOnOff)&&(selectedDigit==1)){LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/1000)*1000))/100)];}	
			if ((digitOnOff)&&(selectedDigit==2)){LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/100)*100))/10)];}
			if ((digitOnOff)&&(selectedDigit==3)){LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[0]-((BVALUE[0]/10)*10)];}		
			//code to blink the selected digit
			if (lastSeconds != seconds)
			{
				if (digitOnOff)
				{
					LED_WhatToDisplay[selectedDigit]=0x00;
					digitOnOff=0;
				}
				else{digitOnOff=1;}
				lastSeconds=seconds;	
			}
			//What the buttons do
			if (!(SW_select_State)) //Select Button moves to the next value and saves at the last value
			{
				SW_select_State=1;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[0]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/1000)*1000))/100)];  
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[0]-((BVALUE[0]/10)*10)];
				if (selectedDigit<3){selectedDigit++;}
				else
				{
					selectedDigit=0;
					digitOnOff=1;
					menuMode=11;
					//CODE HERE TO SAVE TO THE EEPROM
				}
			}
			if (!(SW_Change_State)) //The change button changes the value
			{
				SW_Change_State=1;
				lastSeconds = seconds;
				switch (selectedDigit)
				{
				 case 0:
					if (BVALUE[0]<9000) {BVALUE[0]+=1000;}
					else {BVALUE[0] -= 9000;}
					LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[0]/1000)];		 
				 break;
				 case 1:
					if(((BVALUE[0]-((BVALUE[0]/1000)*1000))/100)<9){BVALUE[0]+=100;}
					else {BVALUE[0]-=900;}				 
				 break;
				 case 2:
					if(((BVALUE[0]-((BVALUE[0]/100)*100))/10)<9){BVALUE[0]+=10;}
					else {BVALUE[0]-=90;}
					LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[0]-((BVALUE[0]/100)*100))/10)];//gets the second digit
				 break;
				 case 3:
					if(BVALUE[0]-((BVALUE[0]/10)*10)<9){BVALUE[0]+=1;}
					else {BVALUE[0]-=9;}
					LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[0]-((BVALUE[0]/10)*10)];	
				 break;						
				}				 									
			}												
		break;
///////////////////////////////NTC2 BETA VALUE - Displays BET2///////////////////////////////////////////////////////////////////
		case 13:
			LED_WhatToDisplay[0] = LED_Display_Char[11];
			LED_WhatToDisplay[1] = LED_Display_Char[14];
			LED_WhatToDisplay[2] = LED_Display_Char[23];
			LED_WhatToDisplay[3] = LED_Display_Char[2];
			//if the Select button is pressed
			if (!(SW_select_State))
			{
				SW_select_State=1;
				menuMode=14;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[1]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/1000)*1000))/100)];
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[1]-((BVALUE[1]/10)*10)];
				selectedDigit=0;
			}
			if (!(SW_Change_State)){SW_Change_State=1;menuMode=15;}		
		break;
///////////////////////////////ADJUST NTC2 BETA VALUE//////////////////////////////////////////////////////////////////////////////////			
		case 14:
			if ((digitOnOff)&&(selectedDigit==0)){LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[1]/1000)];}
			if ((digitOnOff)&&(selectedDigit==1)){LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/1000)*1000))/100)];}	
			if ((digitOnOff)&&(selectedDigit==2)){LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/100)*100))/10)];}
			if ((digitOnOff)&&(selectedDigit==3)){LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[1]-((BVALUE[1]/10)*10)];}		
			//code to blink the selected digit
			if (lastSeconds != seconds)
			{
				if (digitOnOff)
				{
					LED_WhatToDisplay[selectedDigit]=0x00;
					digitOnOff=0;
				}
				else{digitOnOff=1;}
				lastSeconds=seconds;	
			}
			//What the buttons do
			if (!(SW_select_State)) //Select Button moves to the next value and saves at the last value
			{
				SW_select_State=1;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[1]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/1000)*1000))/100)];  
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[1]-((BVALUE[1]/10)*10)];
				if (selectedDigit<3){selectedDigit++;}
				else
				{
					selectedDigit=0;
					digitOnOff=1;
					menuMode=13;
					//CODE HERE TO SAVE TO THE EEPROM
				}
			}
			if (!(SW_Change_State)) //The change button changes the value
			{
				SW_Change_State=1;
				lastSeconds = seconds;
				switch (selectedDigit)
				{
				 case 0:
					if (BVALUE[1]<9000) {BVALUE[1]+=1000;}
					else {BVALUE[1] -= 9000;}
					LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[1]/1000)];		 
				 break;
				 case 1:
					if(((BVALUE[1]-((BVALUE[1]/1000)*1000))/100)<9){BVALUE[1]+=100;}
					else {BVALUE[1]-=900;}				 
				 break;
				 case 2:
					if(((BVALUE[1]-((BVALUE[1]/100)*100))/10)<9){BVALUE[1]+=10;}
					else {BVALUE[1]-=90;}
					LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[1]-((BVALUE[1]/100)*100))/10)];//gets the second digit
				 break;
				 case 3:
					if(BVALUE[1]-((BVALUE[1]/10)*10)<9){BVALUE[1]+=1;}
					else {BVALUE[1]-=9;}
					LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[1]-((BVALUE[1]/10)*10)];	
				 break;						
				}				 									
			}												
		break;
///////////////////////////////NTC3 BETA VALUE - Displays BET3///////////////////////////////////////////////////////////////////
		case 15:
			LED_WhatToDisplay[0] = LED_Display_Char[11];
			LED_WhatToDisplay[1] = LED_Display_Char[14];
			LED_WhatToDisplay[2] = LED_Display_Char[23];
			LED_WhatToDisplay[3] = LED_Display_Char[3];
			//if the Select button is pressed
			if (!(SW_select_State))
			{
				SW_select_State=1;
				menuMode=16;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[2]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/1000)*1000))/100)];
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[2]-((BVALUE[2]/10)*10)];
				selectedDigit=0;
			}
		if (!(SW_Change_State)){SW_Change_State=1;menuMode=17;}
		break;		
///////////////////////////////ADJUST NTC3 BETA VALUE//////////////////////////////////////////////////////////////////////////////////			
		case 16:
	if ((digitOnOff)&&(selectedDigit==0)){LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[2]/1000)];}
if ((digitOnOff)&&(selectedDigit==1)){LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/1000)*1000))/100)];}
		if ((digitOnOff)&&(selectedDigit==2)){LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/100)*100))/10)];}
			if ((digitOnOff)&&(selectedDigit==3)){LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[2]-((BVALUE[2]/10)*10)];}		
			//code to blink the selected digit
			if (lastSeconds != seconds)
			{
				if (digitOnOff)
				{
					LED_WhatToDisplay[selectedDigit]=0x00;
					digitOnOff=0;
				}
				else{digitOnOff=1;}
				lastSeconds=seconds;	
			}
			//What the buttons do
			if (!(SW_select_State)) //Select Button moves to the next value and saves at the last value
			{
				SW_select_State=1;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[2]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/1000)*1000))/100)];  
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[2]-((BVALUE[2]/10)*10)];
				if (selectedDigit<3){selectedDigit++;}
				else
				{
					selectedDigit=0;
					digitOnOff=1;
					menuMode=15;
					//CODE HERE TO SAVE TO THE EEPROM
				}
			}
			if (!(SW_Change_State)) //The change button changes the value
			{
				SW_Change_State=1;
				lastSeconds = seconds;
				switch (selectedDigit)
				{
				 case 0:
					if (BVALUE[2]<9000) {BVALUE[2]+=1000;}
					else {BVALUE[2] -= 9000;}
					LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[2]/1000)];		 
				 break;
				 case 1:
					if(((BVALUE[2]-((BVALUE[2]/1000)*1000))/100)<9){BVALUE[2]+=100;}
					else {BVALUE[2]-=900;}				 
				 break;
				 case 2:
					if(((BVALUE[2]-((BVALUE[2]/100)*100))/10)<9){BVALUE[2]+=10;}
					else {BVALUE[2]-=90;}
					LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[2]-((BVALUE[2]/100)*100))/10)];//gets the second digit
				 break;
				 case 3:
					if(BVALUE[2]-((BVALUE[2]/10)*10)<9){BVALUE[2]+=1;}
					else {BVALUE[2]-=9;}
					LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[2]-((BVALUE[2]/10)*10)];	
				 break;						
				}				 									
			}												
		break;
///////////////////////////////NTC4 BETA VALUE - Displays BET4///////////////////////////////////////////////////////////////////
		case 17:
			LED_WhatToDisplay[0] = LED_Display_Char[11];
			LED_WhatToDisplay[1] = LED_Display_Char[14];
			LED_WhatToDisplay[2] = LED_Display_Char[23];
			LED_WhatToDisplay[3] = LED_Display_Char[4];
			//if the Select button is pressed
			if (!(SW_select_State))
			{
				SW_select_State=1;
				menuMode=18;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/1000)*1000))/100)];
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[3]-((BVALUE[3]/10)*10)];
				selectedDigit=0;
			}
			if (!(SW_Change_State)){SW_Change_State=1;menuMode=19;}
		break;
///////////////////////////////ADJUST NTC4 BETA VALUE//////////////////////////////////////////////////////////////////////////////////			
		case 18:
		if ((digitOnOff)&&(selectedDigit==0)){LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];}
		if ((digitOnOff)&&(selectedDigit==1)){LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/1000)*1000))/100)];}
		if ((digitOnOff)&&(selectedDigit==2)){LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/100)*100))/10)];}
			if ((digitOnOff)&&(selectedDigit==3)){LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[3]-((BVALUE[3]/10)*10)];}		
			//code to blink the selected digit
			if (lastSeconds != seconds)
			{
				if (digitOnOff)
				{
					LED_WhatToDisplay[selectedDigit]=0x00;
					digitOnOff=0;
				}
				else{digitOnOff=1;}
				lastSeconds=seconds;	
			}
			//What the buttons do
			if (!(SW_select_State)) //Select Button moves to the next value and saves at the last value
			{
				SW_select_State=1;
				LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];
				LED_WhatToDisplay[1]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/1000)*1000))/100)];  
				LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/100)*100))/10)];
				LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[3]-((BVALUE[3]/10)*10)];
				if (selectedDigit<3){selectedDigit++;}
				else
				{
					selectedDigit=0;
					digitOnOff=1;
					menuMode=17;
					//CODE HERE TO SAVE TO THE EEPROM
				}
			}
			if (!(SW_Change_State)) //The change button changes the value
			{
				SW_Change_State=1;
				lastSeconds = seconds;
				switch (selectedDigit)
				{
				 case 0:
					if (BVALUE[3]<9000) {BVALUE[2]+=1000;}
					else {BVALUE[3] -= 9000;}
					LED_WhatToDisplay[0]=LED_Display_Char[(BVALUE[3]/1000)];		 
				 break;
				 case 1:
					if(((BVALUE[3]-((BVALUE[3]/1000)*1000))/100)<9){BVALUE[3]+=100;}
					else {BVALUE[3]-=900;}
				 break;
				 case 2:
					if(((BVALUE[3]-((BVALUE[3]/100)*100))/10)<9){BVALUE[3]+=10;}
					else {BVALUE[3]-=90;}
					LED_WhatToDisplay[2]=LED_Display_Char[((BVALUE[3]-((BVALUE[3]/100)*100))/10)];//gets the second digit
				 break;
				 case 3:
					if(BVALUE[3]-((BVALUE[3]/10)*10)<9){BVALUE[3]+=1;}
					else {BVALUE[3]-=9;}
					LED_WhatToDisplay[3]=LED_Display_Char[BVALUE[3]-((BVALUE[3]/10)*10)];	
				 break;						
				}				 									
			}												
		break;															
	}
	//Check if the LED_Display needs updating and send the new data to the display
	if ((LED_WhatisDisplayed[0] != LED_WhatToDisplay[0]) | (LED_WhatisDisplayed[1] != LED_WhatToDisplay[1]) | (LED_WhatisDisplayed[2] != LED_WhatToDisplay[2]) | (LED_WhatisDisplayed[3] != LED_WhatToDisplay[3]))
	{
		sendtoDisplay();
	}
	//Check that the seconds, minutes or hours has not overflowed
	if (seconds > 59){seconds=0; CurrentTime++;}
	if (CurrentTime >= 1440){CurrentTime=0;}	
  }	
}

It´s no problem to add the cases back in if needed. When the code is running normally through the main() loop and a button is pressed, this menu function is run and will continue to run in a loop by the while statement at the beginning. When the code is finished there will be another option to exit the menu and return to normal operation, but i have not done this yet.

Sorry the post is so long.

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

I have kind of fixed the issue. I decided to leave the bug alone for now and continue with the code. When I wrote the next case, the BVALUE[3] case works normally, without the error, but now the new case doesn´t work properly. It is a similar error except the value can be changed but only twice, after that it gets random numbers instead. I am going to keep working for now and get the code finished and then fix the bugs later.

I did change microcontrollers, so that might have something to do with it. I was using an ATMEGA168A and I didn't have enough memory to upload the next case, so I have had to switch to a 328P. I am wondering if it was perhaps a memory issue in the first place?

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

I would suggest rethinking your strategy and fix the error now. It's very easy to end up with code that depends on something working incorrectly and that makes it much more difficult to repair it later. Ideally, errors and warnings are both fixed immediately, unless one has a very good reason for not doing so.

Martin Jay McKee

As with most things in engineering, the answer is an unabashed, "It depends."

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

I would also suggest de-duplicating your code. You have four large blocks of code that are nearly identical which can be reduced to one instance with a little bit of effort (less effort than trying to keep all four copies in sync every time you fix one case).

You should also learn about the modulus operator (%) to extract a single digit. For example, (x/100)%10 gets the digit in the 100's place, and (x/1000)%10 gets the digit in the 1000's place.

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

christop wrote:
I would also suggest de-duplicating your code. You have four large blocks of code that are nearly identical which can be reduced to one instance with a little bit of effort (less effort than trying to keep all four copies in sync every time you fix one case).

I agree, it´s getting silly now a lot of the code is like that. I need to think of a better way to do the menu than what I am trying to do now. When I started, I didn't´t envisage the menu system being as big as it actually is and the way I am doing it now makes it far too easy for me to make a mistake somewhere and miss it.

Quote:

You should also learn about the modulus operator (%) to extract a single digit. For example, (x/100)%10 gets the digit in the 100's place, and (x/1000)%10 gets the digit in the 1000's place.

Thanks, I had no idea that existed, sure does help a lot.

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

Quote:
For example, (x/100)%10 gets the digit in the 100's place, and (x/1000)%10 gets the digit in the 1000's place.
Not exactly.

uint16_t x;
uint8_t thousands,hundreds,tens,units;

x = 3456;
thousands = x/1000;
hundreds  = (x%1000)/100;
tens      = ((x%1000)%100)/10;
units     = ((x%1000)%100)%10;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Visovian wrote:
Quote:
For example, (x/100)%10 gets the digit in the 100's place, and (x/1000)%10 gets the digit in the 1000's place.
Not exactly.

uint16_t x;
uint8_t thousands,hundreds,tens,units;

x = 3456;
thousands = x/1000;
hundreds  = (x%1000)/100;
tens      = ((x%1000)%100)/10;
units     = ((x%1000)%100)%10;


That's pretty clever.

How about the other way around? This method uses the same number of divisions but is more obvious in what it does:

units     = x % 10; x /= 10;
tens      = x % 10; x /= 10;
hundreds  = x % 10; x /= 10;
thousands = x;

But to be fair, (x/100)%10 and (x/1000)%10 do work as advertised, even if they are not as "optimized" as your version.

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

Quote:
(x/100)%10 and (x/1000)%10 do work as advertised

You are right.

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

Thanks, that is really useful information.

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

Quote:

This method uses the same number of divisions but is more obvious in what it does:
Code:
units = x % 10; x /= 10;
...

For a straightforward implementation, especially when tackling something like "my xxyy hurts", the mod plus div is attractive.

Note that internally the mod and div are probably [nearly] identical; one returning the remainder and the other the quotient.

Depending on the toolchain and the phase of the moon and whether a leap year or not, there may be functionality such as div() or ldiv() or divmod() or similar that knows to keep both remainder and quotient. This can make a significant improvement in that type of "binary to display string" algorithms.

https://www.avrfreaks.net/index.p...

Quote:
Quote:
also read up on the modulus % operator.
Using modulus would be wasteful here as it would invoke a second divide. However you can use ldiv() to get both the quot and the remainder in one divide.

https://www.avrfreaks.net/index.p...

Quote:
Koshchi wrote:
Using modulus would be wasteful here as it would invoke a second divide.
Well, you could use an optimizing compiler like avr-gcc.

With the current version of that compiler and optimizing for code size, you'll get
Code:
toHMS:
ldi r18,lo8(60)
ldi r19,0
ldi r20,0
ldi r21,0
rcall __udivmodsi4 ...

Touched on here:
https://www.avrfreaks.net/index.p...

And in the grand-daddy thread:
https://www.avrfreaks.net/index.p...

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.