Why does my while loop only run once in C code?

[Dmeads]

Hello!

New to C programming, and my while loop only runs once in the program. The uart displays the count correctlly, but the count wont restart or move to a different mode (the mode input is coming from an FPGA, this code for the xilinx ZYNQ SoC).

Code: [Select]

#include <stdio.h>
#include <xgpio.h>
#include "xparameters.h"
#include <sleep.h>
#include "platform.h"
#include "xil_printf.h"

#define DEFAULT 1

int main(void){


	XGpio enable,LED;

	int mode;
	int countup;
	int countdown;
	int delay = 900000;

	init_platform();
	printf("counting up!\n\r");
	XGpio_Initialize(&enable,XPAR_AXI_GPIO_0_DEVICE_ID);
	XGpio_SetDataDirection(&enable, DEFAULT, 0xFFFF);

	XGpio_Initialize(&LED,XPAR_AXI_GPIO_1_DEVICE_ID);
	XGpio_SetDataDirection(&LED, DEFAULT, 0x0000);

	while(1){

		mode = XGpio_DiscreteRead(&enable, DEFAULT);

		printf("mode %d", mode);

		if (mode == 1){

			print("counting up!\n\r");

			for(countup = 0; countup <= 15; countup++){
				XGpio_DiscreteWrite(&LED, DEFAULT, countup);
				printf("count %d\n\r", countup);
				usleep(delay);
				if (countup == 16){
					countup = 0;
					}
			}
		}

		else if (mode == 2){

			print("counting down!\n\r");

			for(countdown = 15; countdown >= 0; countdown--){
					XGpio_DiscreteWrite(&LED, DEFAULT, countdown);
					printf("count %d\n\r", countdown);
					usleep(delay);

					if (countdown == -1){
						countdown = 15;
					}
			}
		}

		else{
			XGpio_DiscreteWrite(&LED, DEFAULT, 0);
			print("waiting for user input\n\r");
			usleep(delay);
		}

		return 0;
		cleanup_platform();
	}
}

any thoughts?

Thanks

[hamster_nz]

Look carefully at which block of code there 'return' is in.

That would be my first guess, looking at it on my phone....

[newbrain]

What hamster_nz said.

Plus, there are other part s of code which might not do what you expect, if anything at all.

As a beginner in C, this is not surprising, but luckily many tools will help you detect and correct this kind of problems.
A good editor/IDE will automatically indent the code, in fact, it has (or you did) and the wrong position of the return statement is made clear by its indentation.
A good compiler, with the right options, will then warn you that the statement "cleanup_platform();" has no effect, as it will never be executed after the return.

Then there are the the if statement resetting the variables used for the loops: the condition in the the if will never be verified, so they are not needed.
Moving them outside the loop will cause the if to trigger, but with no practical effect (the variables are not used anywhere else, and are reset by the for statements).

[Siwastaja]

A good compiler, with the right options, will then warn you

[emphasis added]

This; adding -Wall to the gcc command line catches so many beginner mistakes, and stupid mistakes even more experienced programmers do by accident. It's sad that most warnings are not enabled by default.

[hamster_nz]

You need a vastly smaller infinity of monkeys to type the works of Shakespeare if they have autocorrect enabled iPads. 

Learning a new programming language by grabbing a smart IDE and then throwing the resulting code at a dialed-up compiler to catch gross structural errors all seems very post-millennial to me.

(But yes, get a good IDE and do turn on all the compiler warnings)

[emece67]

.

[ghinckley68]

Hello!

New to C programming, and my while loop only runs once in the program. The uart displays the count correctlly, but the count wont restart or move to a different mode (the mode input is coming from an FPGA, this code for the xilinx ZYNQ SoC).

Code: [Select]

#include <stdio.h>
#include <xgpio.h>
#include "xparameters.h"
#include <sleep.h>
#include "platform.h"
#include "xil_printf.h"

#define DEFAULT 1

int main(void){


	XGpio enable,LED;

	int mode;
	int countup;
	int countdown;
	int delay = 900000;

	init_platform();
	printf("counting up!\n\r");
	XGpio_Initialize(&enable,XPAR_AXI_GPIO_0_DEVICE_ID);
	XGpio_SetDataDirection(&enable, DEFAULT, 0xFFFF);

	XGpio_Initialize(&LED,XPAR_AXI_GPIO_1_DEVICE_ID);
	XGpio_SetDataDirection(&LED, DEFAULT, 0x0000);

	while(1){

		mode = XGpio_DiscreteRead(&enable, DEFAULT);

		printf("mode %d", mode);

		if (mode == 1){

			print("counting up!\n\r");

			for(countup = 0; countup <= 15; countup++){
				XGpio_DiscreteWrite(&LED, DEFAULT, countup);
				printf("count %d\n\r", countup);
				usleep(delay);
				if (countup == 16){
					countup = 0;
					}
			}
		}

		else if (mode == 2){

			print("counting down!\n\r");

			for(countdown = 15; countdown >= 0; countdown--){
					XGpio_DiscreteWrite(&LED, DEFAULT, countdown);
					printf("count %d\n\r", countdown);
					usleep(delay);

					if (countdown == -1){
						countdown = 15;
					}
			}
		}

		else{
			XGpio_DiscreteWrite(&LED, DEFAULT, 0);
			print("waiting for user input\n\r");
			usleep(delay);
		}

		return 0;
		cleanup_platform();
	}
}

any thoughts?

Thanks

ok get rid of return 0;  the return is the exit from the function. Same on most os and compilers not sure about the xlinx, but return  means exit from the function and return a 0 to the calling block of code, in this case that would probally be the xlixs, nothing else runs after the return.