Function that is not working

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

Hello all!

In spite of being registered some time ago, most of what I have done was in assembly. I am trying to learn C, but it has been difficult to me.

I wrote a function to replace the code below. I have a function to send the "pressure_byte" to USART, but, I want send it with other data I have in a vector.

   			pressure_byte = (uint8_t )(*ptr >> 16 ); // get 3 MSBs 
            send_byte(pressure_byte); 

  			pressure_byte = (uint8_t )(*ptr >> 8 ); // Next 8 bits 
            
        	send_byte(pressure_byte); 

			pressure_byte = (uint8_t )(*ptr ); //8 LSBs 
            
        	send_byte(pressure_byte);

The function I wrote is below and I want do save the pressure byte in a vector.

void save_pressure(uint32_t  *ptr)
 {
	ptr=&pressure;

	
	int j=32;
	int i;

	for (i=16; i=0; i-8)
	 	{
		GPS_data[j] = (uint8_t )(*ptr >> i ); // get 3 MSBs 
        j=j+1;
		}
  			 // Next 8 bits 
            
			 //8 LSBs ( shouldn't need the cast ) 
            
 }

The function is called by this:


void save_pressure(uint32_t *ptr);

the function is declared like this:

void save_pressure(uint32_t *ptr);

I have tried several ways but I do not write any thing to the vector.

your help is appreciated.

Thanks in advance.~
best regards,
Manuel Silva

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

Not enough info and what you have posted for both "called by this" and "declared like this" is the same thing (the declaration). I'd like to see how you ARE calling it.

Also, inside the function what do you see as the point in passing in a parameter to 'ptr' then immediately over-writing that variable? If you are going to do that then forgot the parameters and just use globals.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
void save_pressure(uint32_t  *ptr)
 {
   ptr=&pressure;

Why do you pass a pointer as argument to the function, when the first thing you do in that function is to overwrite that pointer?

   for (i=16; i=0; i-8) 

This loop never iterates. What you actually mean is this (look in your C book for the details):

   for (i=16; i>0; i-=8) 

But the whole loop is quite pointless for me (unnecessarily complicated). Why don't you simply write:

GPS_data[32] = (uint8_t )(*ptr >> 16);
GPS_data[33] = (uint8_t )(*ptr >> 8);

That's what the loop do with the corrected for-line.

Stefan Ernst

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

Thanks to both of you.

I agree that the way you shown is much simpler.


GPS_data[32] = (uint8_t )(*ptr >> 16); 
GPS_data[33] = (uint8_t )(*ptr >> 8); 

thanks,
Best regards,
Manuel Silva

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

That still does not resolve your issue with parameter passing. Even assuming you didn't over-write the parameter I simply don't see why you are passing by reference rather than passing by value - all you do in the function is read *ptr anyway. So why not just pass in the uint32_t value? You usually use pointers for parameters when either they are an array of objects or you plan to update the values located at the referenced address.

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

Clawson, I already solved the problem using the idea of sternst, but, really I would like to understand how doing it with a function.

As I told before, I am starting learn C, so, even the language you are using is a bit difficult to understand.

The program I am doing is "trial and error", read a the C book and try it. If it not work I try again.

Can you give me a small example on how do it?

Thanks and regards,
Manuel Silva

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

Quote:

Can you give me a small example on how do it?

From what you have given us so far I'm still not clear what you are trying to do. Usually when C code is quoted it gives an idea of what's required even if there's the odd syntax error or something but your use of function parameters makes no sense so I really have no idea what you are trying to achieve.

Either explain in words, psuedo code or some kind of flowchart or other design diagram/document.

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

Clawson,

Below is the main of the C program I am trying to write. I am using the SCP1000 pressure sensor code example to read the altitude of a model airplane. I am also trying to include the reading of battery voltage, rotations per minute, GPS data, air speed, motor temperature. All of this is already working. Now I am trying to include battery miliwhats discharge (i do not know if I will be able to do it due timming problems). All of this data will be sent down using a two way radio control system. the data is send as row data (battery voltage = ADC reading 2 bytes) and the base do the calculations checks the alarm levels displays it.

As I said here is the main:

int main(void)
{
	cli();
	wait_ms(10);
	uint16_t temp;
	uint32_t pressure;
	uint32_t *ptr; 	// must be the same zise as pressure	
	uint16_t adc_value;
	setup(); // #1, configure MCU

	initialize_usart();

	//UCSR1B |=(1<<RXCIE1);

	sei();

	init_scp1000(); // #2, Initialize SCP1000, activate
			// 'high resolution' measurement mode
  
	for(;;)
	{
	ReadGPS_data();
	EIMSK = 0x01; // enable INT0
			

	while(!(SCP1000_PIND & (1 << SCP1000_DRDY)))	// Wait for DRDY pin='1'
	wait_ms(1);
	temp = Read_Direct_Access_SPI(0x21, 2);			// Read 2 bytes from TEMPOUT (0x21)
				temp = temp & 0x3fff;							// Mask bits [13:0]
	temp = temp / 20; // Convert temperature to [°C] by dividing
			 // the result by 20

	pressure = (uint32_t)((0x0007) & Read_Direct_Access_SPI(0x1F, 1));
																// Read 1 byte from DATARD8 (0x1F) and
																// mask bits [2:0] in order to get
																// 3MSB bits for pressure information
				pressure <<= 16; 								// shift 3MSB bits to left by 16
				pressure |= (uint32_t)Read_Direct_Access_SPI(0x20, 2);
																// Read 2 bytes from DATARD16 (0x20) in order to get 16 LSB bits for pressure
																// information.
				pressure >>= 2; 								// Convert to [Pa] by dividing the 19 bit pressure by 4 --> the 19 bit pattern
																  // is shifted to right by 2
																// <- Here user can add temperature and pressure data send to display routines
																// as needed for application. Notice that temperature is in 2's complement format.


				ptr=&pressure;

									//
   	GPS_data[32] = (uint8_t )(*ptr >> 16 ); // get 3 MSBs 
         GPS_data[33] = (uint8_t )(*ptr >> 8 ); // Next 8 bits 
         GPS_data[34] = (uint8_t )(*ptr ); //8 LSBs ( shouldn't need the cast ) 
            
     

		
			
			
		
	 adc_value = Read_ADC(0);
	 save_adc_data(35, adc_value);

	 adc_value = Read_ADC(1);
	 save_adc_data(37, adc_value);

	 adc_value = Read_ADC(2);
	 save_adc_data(39, adc_value);

	 adc_value = Read_ADC(3);
	 save_adc_data(41, adc_value);
		
	Test_Data();


		}
return 0;
}

What I was trying to replace with a function was this:

     GPS_data[32] = (uint8_t )(*ptr >> 16 ); // get 3 MSBs 
         GPS_data[33] = (uint8_t )(*ptr >> 8 ); // Next 8 bits 
         GPS_data[34] = (uint8_t )(*ptr ); //8 LSBs ( shouldn't need the cast ) 

Please let me know if you want any particular code like functions for better understanding.

Thanks in advance,
Best regards,
Manuel Silva

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

Given that main function we can return to the questions that have been asked previously, "Why were you passing in a pointer and why were you re-assigning it immediately." To that we can add another question, "Where was that function defined." You do not show where GPS_Data was defined, but, as it is not in the main function, I shall assume that it is global - no problem there. However, the variable pressure is defined within the main function. In C you cannot define a function within another function ( yes, gcc has that as an extension, we will ignore that for now ). If you defined save_pressure outside of main then it will not "see" the variable pressure and it should give you an error on compilation. If there is a variable named pressure in global scope it will compile without warning, but it will be a different variable ( because it is in a different scope ), and so it will not write the values you are expecting.

If, as you say, sternst's code is working as you would like it to ( and looking at the original code that is storing 3 bytes ), you can wrap it as a function like this,

void save_pressure( uint32_t _pressure ){
	GPS_data[32] = (uint8_t )( _pressure >> 16 );
	GPS_data[33] = (uint8_t )( _pressure >> 8 );
	GPS_data[34] = (uint8_t )( _pressure ); 	
	return;
}

and in main call it as,

save_pressure( pressure );

Martin Jay McKee

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

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

As Martin's example shows:

            ptr=&pressure;

      GPS_data[32] = (uint8_t )(*ptr >> 16 ); // get 3 MSBs
         GPS_data[33] = (uint8_t )(*ptr >> 8 ); // Next 8 bits
         GPS_data[34] = (uint8_t )(*ptr ); //8 LSBs ( shouldn't need the cast

is identical to:

      GPS_data[32] = (uint8_t )(pressure >> 16 ); // get 3 MSBs
         GPS_data[33] = (uint8_t )(pressure >> 8 ); // Next 8 bits
         GPS_data[34] = (uint8_t )(pressure ); //8 LSBs ( shouldn't need the cast

The use of the pointer was completely pointless.

I think you are possibly confusing two different ways to split a uint32_t into several bytes. If you wanted to use a pointer then the following would have worked:

uint8_t * ptr;
ptr = (uint8_t *)&pressure;
         GPS_data[32] = *(ptr + 2); // get 3 MSBs
         GPS_data[33] = *(ptr + 1); // Next 8 bits
         GPS_data[34] = *ptr; //8 LSBs ( shouldn't need the cast

But this is not as "good" as the bit shifts as it is reliant on endianness.

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

Mckeemj, Clawson, thanks for the inputs. As I told before I am trying to learn C, so I am reading the C book, trying, if it doesn't work read again, etc.

The main point here is, I understood the way you both did the changes.

Thanks and regards,

Manuel Silva