CODEWITHSUNDEEP
cmodulus
Chef Flambe
Chef Flambe

Reputation: 883

modulus operator with unsigned chars

Trying to get some code working and the modulus doesn't want to do what I want it to do... which means I have it wrong.

I have unsigned chars that I'm trying to seperate out hours/minutes/seconds into so I can display them on the screen in Ascii.

The variable secs is anunsigned int. Everything else is anunsigned char. I want the results in unsigned chars so not to waste memory. Working in an embedded environment.

Anyone care to look at the code snippet and tell me what I've done wrong?

hours   = secs/3600.0;
minutes =(secs/60.0)-(hours*3600);
seconds =secs-(hours*3600)-(minutes*60);

sec_ones    =(unsigned char)((seconds%10));
sec_tens    =(unsigned char)((seconds-sec_ones)%100);
min_ones    =(unsigned char)(minutes%10);
min_tens    =(unsigned char)((minutes-min_ones)%100);
hrs_ones    =(unsigned char)(hours%10);
hrs_tens    =(unsigned char)((hours-hrs_ones)%100);

Upvotes: 2

Views: 1706

Answers (4)

Steve Jessop
Steve Jessop

Reputation: 279385

minutes =(secs/60.0)-(hours*3600);

should be

minutes =(secs/60.0)-(hours*60);

Other than that, it works for small enough input: http://ideone.com/VPKP1

There are some things that I'd change, though. For example, there's no point doing a double division and then assigning the result back to unsigned char, you might as well just do integer division.

Upvotes: 2

ouah
ouah

Reputation: 145899

You mentioned it is an embedded program.

seconds = secs-(hours*3600)-(minutes*60);

hours is an unsigned char promoted to int in the expression hours*3600.

If your are working with 16-bit int you will have problems with the above line. On two's complement system 16-bit int range go from -32768 to 32767 which mean you have an overflow when hours is >= 10.

Upvotes: 1

elhadi dp ıpɐɥןǝ
elhadi dp ıpɐɥןǝ

Reputation: 5221

your program is just bad written, try to do that, it run correctly

unsigned int secs = 5000;
unsigned char sec_ones,sec_tens,min_ones,min_tens,hrs_ones,hrs_tens, hours, minutes, seconds;

hours   = secs/3600.0;
minutes =(secs/60.0)-(hours*60);
seconds =secs-(hours*3600)-(minutes*60);

sec_ones    =(unsigned char)((seconds%10));
sec_tens    =(unsigned char)(seconds/10);
min_ones    =(unsigned char)(minutes%10);
min_tens    =(unsigned char)(minutes/10);
hrs_ones    =(unsigned char)(hours%10);
hrs_tens    =(unsigned char)(hours/100);

printf("%.1u%.1u:%.1u%.1u:%.1u%.1u\n",hrs_tens,hrs_ones,min_tens,min_ones,sec_tens,sec_ones );

Upvotes: 0

Jens Gustedt
Jens Gustedt

Reputation: 78973

first, you do your computations with double since you are using double constants.

then the modulus calculations will not be done as unsigned char because it is a narrow type. generally it will first be promoted to int and then the calculations will be done.

Usually you are better off by using unsigned int directly for all your variables. char types are only useful when you want to save space.

Upvotes: 0

Related Questions