-3

I have a problem with modulo arithmetics in C language. I have defined global variable uint16_t Tmr_1ms which is incremented every 1 ms. I want to use this variable in a software oscillator implementation which is realized in below given function

 void OSC(uint32_t output, Osc_t *p){

    float act_time;

    if(p->start){

        taskENTER_CRITICAL();
            act_time = Tmr_1ms;
        taskEXIT_CRITICAL();


        if(p->init){

            SetLogicSignal(output);
            p->start_time = act_time;
            p->delta = ((p->period)/(2*p->T));
            p->init  = FALSE;

        }

        if(((uint16_t)act_time - (uint16_t)(p->start_time)) >= ((uint16_t)(p->delta))){

            NegLogicSignal(output); // my defined function for negation of a bit variable
            p->start_time = act_time;

        }

    }else{

        ClearLogicSignal(output);
        p->init = TRUE;

    }

}

The oscillator state is stored in an instance of below given structure

    // oscillator state (oscillator with fixed duty cycle)
typedef struct{
    float period;         // period of the oscillations (ms)
    float T;              // function execution period (ms)
    BOOL start;           // oscillator start signal (start==TRUE, stop==FALSE)
    BOOL init;            // initiate the oscillator state
    float delta;          // time after which expiration the oscillator output is negated
    float start_time;     // captured Tmr_1ms value
}Osc_t;

Here is the code

// oscillator instance init
Test_Oscillator_state.T = 20;
Test_Oscillator_state.period = 1000;
Test_Oscillator_state.init = TRUE;

// calling the function
Test_Oscillator_state.start = TRUE;
OSC(LTestBlink, &Test_Oscillator_state);

The problem is in the following code

    if(((uint16_t)act_time - (uint16_t)(p->start_time)) >= ((uint16_t)(p->delta))){

            NegLogicSignal(output);
            p->start_time = act_time;

}

The output negation is functional only before Tmr_1ms overflow. I don't understand why. Please can anybody give me any guidance? Thanks in advance.

Steve
  • 805
  • 7
  • 27
  • 1
    Provide a [mcve] and follow [ask[. What do you mean with "funcional"? There is no negation operator and `NegLogicSignal` is not a standard function. Learn to use the debugger. – too honest for this site Sep 04 '17 at 12:08
  • there isn't any modulo operator in your code, so unsure what modulo arithmetic you're having problems with? – Chris Turner Sep 04 '17 at 12:10
  • I am not at all sure what your question/problem is. Do you mean that once Tmr_1ms wraps around you never again get into the "if" statement to call "NegLogicSignal(output)"? – Basya Sep 04 '17 at 12:12
  • 2
    why are you casting floats to integers here? What are you trying to accomplish with that? – Basya Sep 04 '17 at 12:15
  • The problem is exactly in not achieving the NegLogicSignal(output) function call. The intention of casting floats to integers was to exploit the modulo arithmetic. – Steve Sep 04 '17 at 12:21
  • When you say, "Tmr_1ms overflow", do you mean it is overflowing the float (sounds less likely) or that it has a value that overflows the 16 bit integer to which you cast its output? – Basya Sep 04 '17 at 12:24
  • I meant that the global variable Tmr_1ms which is unsigned 16 bit variable achieved the 65536. – Steve Sep 04 '17 at 12:27
  • 3
    so why are you using floats at all? If you are using a 16-bit time, stick with unsigned 16-bit numbers! Converting to float and back doesn't look right to me. – Basya Sep 04 '17 at 12:40
  • Avoid negative numbers: `if((uint16_t)act_time >= ((uint16_t)(p->delta)) + (uint16_t)(p->start_time)){` Use `(uint32_t)(p->start_time)` if `unsigned` is 16-bit. – chux - Reinstate Monica Sep 04 '17 at 14:47
  • 1
    floating point is certainly not required here. do your math like this on a calculator pre-coding time, not runtime, ideally not compile time although if you do it right you can code it compile time and not incur the massive penalty. – old_timer Sep 04 '17 at 15:21
  • note modulo in C is simply taking the remainder of a division a = b / c; a is the quotient. d = b % c; d is the remainder. floating point is not required. although when computing how many timer clock ticks make up a millisecond floating point nor modulo is really required. if you want fraction of the period for some reason fixed point works fine, depends on your real goal. as pointed out below subtracting start from finish or vice versa (depending on if it is a count up or down) is perfectly fine with some rules attached. – old_timer Sep 04 '17 at 15:24

1 Answers1

1

Subtracting start time from act time, when act time wraps around, is problematic. You are subtracting a larger unsigned number from a smaller one, which is unlikely to give you what you want. The difference would be negative if these were signed numbers; in unsigned numbers you will get something bit-equivalent to a negative number which will be a large unsigned number (but apparently still smaller than your previously saved start value).

You need to detect and handle the wraparound. Either have another register type value to indicate the wraparound (which clears on read, or you clear when you read it), or have a function which computes the delta note that the start value is closer than delta to the max value. Then the function to compute the difference figures out the correct difference.

Since you have put the values in float variables, you could not cast to unsigned int, and then after the wraparound you'll get a negative number, clearly indicating the wraparound and allowing you to calculate the correct difference.

Take a look at this discussion of unsigned subtraction, it has further explanation and suggestions.

The best solution old_timer suggested in the comments (old_timer, if you want to make it an answer and have it accepted, I'll remove this from my answer). Promote the 16-bit unsigned values to 32-bit unsigned values (float, as done in the original code, is possible but not needed or recommended). Do the subtraction with the 32-bit (or float) values. After the subtraction, mask the bits back to 16-bit (bitwise-and with 0xFFFF or assign to an unsigned 16-bit variable). The subtraction then works, because the arithmetic is done in 32-bits (or float) which will not wrap around. The "modulo" effect is gotten by masking off the higher bits.

Basya
  • 1,477
  • 1
  • 12
  • 22
  • Thank you Basya Perlman for your answer. I have written similar program where the Tmr_1ms variable has been replaced by the state of 16 bit hardware timer/counter. I read the counter value at the beginning of some task (value stored in uint16_t prev_state variable) and then I read counter value at the end of the task (value stored in uint16_t cur_state variable). Following statement returns appropriate results diff = cur_state - prev_state (where diff is uint16_t). Please can you tell me why in this case the subtraction returns appropriate results. Thanks. I am an autodidact. – Steve Sep 04 '17 at 14:04
  • I'll think about it some more, but on first glance, no, I can't tell you why it works :-) -- I wouldn't expect it to, unless in that case you never got to the wraparound. – Basya Sep 04 '17 at 14:16
  • Based on the results with hardware timer/counter I have used the same approach with "software timer" Tmr_1ms. I have expected that for example 12-65528=20 due to unsigned variables. The hardware timer/counter was configured as free-running. Now I am confused. – Steve Sep 04 '17 at 14:21
  • It makes no difference if you are reading a software counter or a hardware counter. I would not expect 12-65528=20. I have had to handle wraparound/overflow many times with hardware counters/timers. – Basya Sep 04 '17 at 14:22
  • Maybe in the other case, where you read the counter twice, it was unlikely (maybe you were just lucky?) to have the wraparound happen exactly then. You can see https://stackoverflow.com/questions/7221409/is-unsigned-integer-subtraction-defined-behavior for answers explaining how unsigned subtraction works (don't only look at the accepted answer). One of the answers gives a suggestion using abs on a signed result, which may work for you. – Basya Sep 04 '17 at 14:27
  • 1
    with a free running timer it is perfectly accurate to subtract start from finish, mask with the number of bits in the timer and get the exact answer. So long as you dont let the timer run two rollovers basically, pass your start number even once. you can roll over all ones to zeros or zeros to all ones and the math is correct, but you pass your start number and you are off by one or more whole rollover periods... – old_timer Sep 04 '17 at 15:19
  • if this is not a free running timer then the math can be made right but more work rolls over at 1234 to 0 or vice versa, can still sort it out but use it as a period timer and look for the roll over dont subtract before and after. – old_timer Sep 04 '17 at 15:20
  • Thanks for your reaction old_timer. So you mean that the problem why above given code in OSC function doesn't work as expected is caused by the fact that more than one rollover of Tmr_1ms occure between two consecutive readings of Tmr_1ms? – Steve Sep 04 '17 at 17:19
  • Steve, I think that old_timer's solution suggests doing the math with more bits than the timer. Then there won't be rollover in your arithmetic and you can then mask the bits to get the right answer. I like that solution. But it is not what you are doing; you are doing the math with the same number of bits as the timer. And, as he says, it won't help you if you roll over twice (hopefully unlikely - if it can reasonably happen in your application than all the solutions proposed here are incomplete). @old_timer, did I understand you correctly? I had to read it a few times :-) – Basya Sep 04 '17 at 19:04
  • @old_timer: can you elaborate on the requirement of a free running timer for your solution? – Basya Sep 04 '17 at 19:53
  • 1
    I think you understood correctly your statement about subtracting being problematic is what I was commenting on, depending on what rollover means the math is not problematic. But you really have to do your design right the timer slow enough to not count past your start time with the number of bits you have, etc...and/or allow for that and keep score with an interrupt or polling of how many times that happened, etc...I dont think the OPs issue has to do with timing anyway but something more fundamental that float just overcomplicates. – old_timer Sep 04 '17 at 21:58
  • 1
    I have tried following. I have defined temporary variable uint32_t elap_time and I have done the subtraction in this manner: elap_time = ((act_time - p->start_time) & 0xFFFF). It seems to be functional. I have also tried following implementation. I have defined temporary variable uint16_t elap_time and I have done the subtraction in this manner: elap_time = (act_time - p->start_time). It seems to be also functional. Please can you tell me whether these implementations are correct. Thanks. – Steve Sep 05 '17 at 06:30
  • 1
    With both act_time and p-> start_time as floats both should be correct. Since you are doing integer arithmetic, though, I'd promote to 32 bit uints and skip the floats entirely -- you don't need them. Stick with integer arithmetic when you can. – Basya Sep 05 '17 at 07:06
  • @BasyaPerlman when I said free-running I was implying it rolls over from all ones to all zeros or all zeros to all ones and for some timers that is not how they work, but usually that is what the term implies, so should have been more clear – old_timer Sep 05 '17 at 15:20
  • @old_timer: got it, thank you! I was assuming that without thinking about it :-) – Basya Sep 05 '17 at 16:28
  • Unless the system is 8-bit or 16-bit, _the calculation is already done on 32 bit signed integers_. `(uint16_t)act_time` etc will get implicitly promoted to type `int`. See [Implicit type promotion rules](https://stackoverflow.com/questions/46073295/implicit-type-promotion-rules). – Lundin Sep 07 '17 at 07:49