Prev: Using pointer-to-member of incomplete type in constructor call in VS2008
Next: filename string manipulation
From: Robby on 4 Feb 2010 15:15
Hello,

Igor, I hope you are still around, I would really like to persue what we
were talking about in the previous thread. I know this seems like we are
going backwards here, but the trouble I am having has more to do on structure
than coding... and ofcourse my lack of experience in C ! :-)

I hope all this will lead up to what I am looking for. Sometimes I have a
hard time putting this stuff into words.

Okay, I like your proposition of the two functions. So let me start with
that. There is a first problem that I am running into. I did the two
functions. The first one called set_curr_sys_osc() and the second one called
get_curr_sys_osc(). Here is what I did so far, I will try to keep it short:

=============================test.h
#ifndef TEST_H
#define TEST_H

#define COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv) \
(((float)(extCrys/fpllidiv)*(float)(fpllmul/fpllodiv))*1000000)

extern static float the_sys_osc; //< store the SYS_CURR_OSC value here!
#endif // TEST_H //

=================================test.c
#include <stdio.h>
#include "test.h"

// In exactly one .c file.
static float the_sys_osc=0;

void set_curr_sys_osc(float extCrys, float fpllidiv, float fpllmul, float
fpllodiv)
{
the_sys_osc = COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv);
}

int get_curr_sys_osc()
{
return (int) the_sys_osc;
}

int main()
{
set_curr_sys_osc(8.0, 2.0, 21.0, 8.0);

// >>> SYSTEMConfig() is not one of my functions!
//SYSTEMConfig((int)get_curr_sys_osc(), SYS_CFG_ALL);

return 0;
}
=======================================

The above code generates the following error:

1>c:\_dts_programming\pic\_microchip_issues\simulated in
vc++\define_macros\define_macros\define_macros\test.h(13) : error C2159: more
than one storage class specified

It seems to point to the extern value I created. I did this extern variable
like all the others.... meaning:

- declaring it in the header file
- innitializing it in exactly one .c file
- using it in a function of test.c

Why the error?

Please get back!

--
Sincere regards
Roberto
From: Igor Tandetnik on 4 Feb 2010 15:25
Robby <Robby(a)discussions.microsoft.com> wrote:
> Okay, I like your proposition of the two functions. So let me start
> with that. There is a first problem that I am running into. I did the
> two functions. The first one called set_curr_sys_osc() and the second
> one called get_curr_sys_osc(). Here is what I did so far, I will try
> to keep it short:
>
> =============================test.h
> #ifndef TEST_H
> #define TEST_H
>
> #define COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv) \
> (((float)(extCrys/fpllidiv)*(float)(fpllmul/fpllodiv))*1000000)

Why do you want this macro in the header file? Are users of this module expected to use it? In fact, it's not clear to me why you need this macro in the first place.

On the other hand, you are talking about the two functions - but you didn't place _their_ declarations into the header. How are clients supposed to call them?

> extern static float the_sys_osc; //< store the SYS_CURR_OSC value
> here! #endif // TEST_H //

There ain't no such thing as "extern static". The two qualifiers have precisely opposite meaning are cannot be combined.

The variable is an internal implementation detail, it shouldn't be in the header.

> =================================test.c
> #include <stdio.h>
> #include "test.h"
>
> // In exactly one .c file.
> static float the_sys_osc=0;
>
> void set_curr_sys_osc(float extCrys, float fpllidiv, float fpllmul,
> float fpllodiv)
> {
> the_sys_osc = COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv);
> }
>
> int get_curr_sys_osc()
> {
> return (int) the_sys_osc;

Why do you store the value as float when you always cast it to int anyway?

> }
>
> int main()
> {
> set_curr_sys_osc(8.0, 2.0, 21.0, 8.0);
>
> // >>> SYSTEMConfig() is not one of my functions!
> //SYSTEMConfig((int)get_curr_sys_osc(), SYS_CFG_ALL);

get_curr_sys_osc already returns int, no need to cast.

> The above code generates the following error:
>
> 1>c:\_dts_programming\pic\_microchip_issues\simulated in
> vc++\define_macros\define_macros\define_macros\test.h(13) : error
> C2159: more than one storage class specified

Like I said, no such thing as "extern static".
--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925

From: Robby on 4 Feb 2010 16:03
> The above code generates the following error:
>
> 1>c:\_dts_programming\pic\_microchip_issues\simulated in
> vc++\define_macros\define_macros\define_macros\test.h(13) : error C2159: more
> than one storage class specified
>
> It seems to point to the extern value I created. I did this extern variable
> like all the others.... meaning:
>
> - declaring it in the header file
> - innitializing it in exactly one .c file
> - using it in a function of test.c
>
> Why the error?

I got it to work, I had to change:

extern static float the_sys_osc;

to

extern float the_sys_osc;

I will continue with the two functions resolution and post back if there is
problems.
Please watch this thread. Thanks!

Rob
From: Robby on 4 Feb 2010 17:28
Igor, you have done it again,

You have inspired me to persist on a resolution you provided and I finally
got it to work. I really, really want to thank you very much.

>Why do you want this macro in the header file? Are users of this module expected >to use it?

Hum, these the questions I am not too clear about yet. But I don't know yet
if users of this module will need to use it... I mean now that we have done
the three functions... its not clear to me either. Perhaps, one might need to
re-compute the current system oscilation value from the macro _again_ !!!

>In fact, it's not clear to me why you need this macro in the first place.

Yeah your right, but I use it in one of the functions, and I need practice
with these macros. I know, its a cheap excuse. :-)

>There ain't no such thing as "extern static". The two qualifiers have precisely >opposite meaning are cannot be combined.

Understood. I fixed it.

In any case Igor, I don't know if what I have done can be improved... by
your or this forum's standars, I suppose so, but I am quite happy with what
I have done. It all runs perfectly in MPLAB, so I haven't tried it in VC but
it does compile without error. So all I did was copy and paste from MPLAB IDE
to VC IDE. Here you can see why sometimes things are hard for me to explain
because I am thinking of everything else that goes on at the same time.
Please view what I have done, and if there is any other recomendations (I am
sure you will have!!!) let me know. But as I say, you have very much helped
me a great deal and I am happy with this so far.

The idea was to store the system frequency clock in a variable and then use
that variable to calculate a precise delay time in milli-seconds or
micro-seconds by simply calling a delay function on the fly. Even though
there are better ways to do delays, when bit banging at very short delays,
calling a delay function on the fly is very convinient. I have created 2 of
these delay functions, one for milli-seconds and another for micro-seconds.
Now, even if I decide to speed up the processor, these delay functions will
take into account all the overhead instructions and still carry out the
correct delays.

Here is the code:

========================KERNEL.h
#ifndef KERNEL_H
#define KERNEL_H

#define COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv) \
(((float)(extCrys/fpllidiv)*(float)(fpllmul/fpllodiv))*1000000)

extern float curr_sys_osc;

void set_curr_sys_osc(float extCrys, float fpllidiv, float fpllmul, float
fpllodiv);
int get_curr_sys_osc();
float get_osc_period();

#endif // KERNEL_H //

=========================KERNEL.c
#include <stdio.h>
#include "KERNEL.h"
#include "MYLIB.h"

// In exactly one .c file.
static float curr_sys_osc=0;

void set_curr_sys_osc(float extCrys, float fpllidiv, float fpllmul, float
fpllodiv)
{curr_sys_osc = COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv);}

int get_curr_sys_osc()
{return (int) curr_sys_osc;}

float get_osc_period()
{return (1/curr_sys_osc);}

int main()
{
set_curr_sys_osc(8.0, 2.0, 21.0, 8.0);
SYSTEMConfig(get_curr_sys_osc(), SYS_CFG_ALL); // Config wait states

// do something...

// Now I can call a delay routine on the fly
// no matter what I set my system frequency to.
delay_us(100);

// other code....
return 0;
}

=========================MYLIB.h
#ifndef MYLIB_H
#define MYLIB_H

#include "KERNEL.h"

// MILLI-SECOND DELAY FUNCTION @ optimization :0:
#define MILLI_SEC 0.001

// MICRO-SECOND DELAY FUNCTION @ optimization :0:
#define MICRO_SEC 0.000001

// INSTRUCTION CYCLES REQUIRED FOR DELAY FUNCTIONS @ optimization :0:
#define OVERHEAD_IC 67 // 67 overhead instructions
#define DELAY_LOOP_IC 8.0 // Instructions in the delay loop + ext # brch
in loop
#define LOOPS_REMOVE(a,b) (a/b) // Calculates dleay loops to remove
#define RATIO(a,b,c) (a/(b*c)) // Figures out ratio to apply to scale freq

void delay_ms(unsigned int ms_delay);
void delay_us(unsigned int us_delay);

#endif //MYLIB_H

===============================MYLIB.c
#include "MYLIB.h"

void delay_us(unsigned int us_delay)
{
unsigned int t;
float x,y;

x = ((float)RATIO((float)MICRO_SEC, get_osc_period(), (float)DELAY_LOOP_IC))
* (float)us_delay; //Ratio constant @ x-MHZ!

// Test if < shortest time possible !
if(x < (LOOPS_REMOVE((float)OVERHEAD_IC, (float)DELAY_LOOP_IC)))
t = 1; // Sys Osc is too slow... apply minimum delay!
else
{
// Get number of while loops to remove
y = (float) LOOPS_REMOVE((float)OVERHEAD_IC, (float)DELAY_LOOP_IC);

// Remove amount of while loops to compensate by time lost by overhead
//instructions
t = (int)(x-y);
}

while(--t) // do the delay!
{}
}


void delay_ms(unsigned int ms_delay)
{
unsigned int t;
float x,y;

x = ((float)RATIO((float)MILLI_SEC, get_osc_period(), (float)DELAY_LOOP_IC))
* (float)ms_delay;

// Test if < shortest time possible !
if(x < (LOOPS_REMOVE((float)OVERHEAD_IC, (float)DELAY_LOOP_IC)))
t = 1; // Sys Osc is too slow... apply minimum delay!
else
{
// Get number of while loops to remove
y = (float) LOOPS_REMOVE((float)OVERHEAD_IC, (float)DELAY_LOOP_IC);

// Remove amount of while loops to compensate by time lost by overhead
// instructions
t = (int)(x-y);
}

while(--t) // do the delay!
{}
}

=========================================

I am sure that more can be done to improve the MYLIB.c/.h module. But for
now, I will leave it like this.

Igor, I thank you for your insight, advice and most imporatant, your
constant assistance you give me time after time.

Sincere regards
Roberto

From: Ulrich Eckhardt on 5 Feb 2010 03:07
Robby wrote:
> #define COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv) \
> (((float)(extCrys/fpllidiv)*(float)(fpllmul/fpllodiv))*1000000)

Why? Why isn't this a function?

> extern float curr_sys_osc;

This says: There is a global variable for anyone to access. However, zero
documentation on how to use it...

> void set_curr_sys_osc(float extCrys, float fpllidiv,
> float fpllmul, float fpllodiv);
> int get_curr_sys_osc();
> float get_osc_period();

All these seem to have something to do with the above global variable.

> // In exactly one .c file.
> static float curr_sys_osc=0;

This says: Create a variable accessible _only_ in this file. This conflicts
with the declaration in the header.

> void set_curr_sys_osc(float extCrys, float fpllidiv, float fpllmul, float
> fpllodiv)
> {curr_sys_osc = COMPUTE_SYS_OSC(extCrys, fpllidiv, fpllmul, fpllodiv);}

The macro is used here, where else? I guess nowhere, so move it into this
source file. Further, since even here it seems to only be used once, so you
don't need it at all. Macro are a simple text substitution mechanism
without any type checking. Adding all the casts you have in there, you get
zero checking. This is a recipe for disaster and bad programming.

> int get_curr_sys_osc()
> {return (int) curr_sys_osc;}
>
> float get_osc_period()
> {return (1/curr_sys_osc);}

Why not get_curr_sys_osc_period()? Why not get_curr_sys_osc_frequency()?
Also, these in combination with set_curr_sys_osc() show that curr_sys_osc
should not be global and visible everywhere.

Mylib.c/h are the same, too many global macros, too much type casting.

Uli

--
C++ FAQ: http://parashift.com/c++-faq-lite

Sator Laser GmbH
Geschäftsführer: Thorsten Föcking, Amtsgericht Hamburg HR B62 932
 |  Next  |  Last
Pages: 1 2 3 4
Prev: Using pointer-to-member of incomplete type in constructor call in VS2008
Next: filename string manipulation