Elapsed time is not giving me expected result

Question

I receive a string containing "1" every few seconds. I am trying to get the elapsed time between the received string and the next one to return the elapsed time. I am doing something wrong. The result I get is 0 and the string is instead updated exactly every second, so I should read 1. I am quite sure there is a mistake in the logic but I cannot see where it is. This should run for hours and update every time I get an update on the string "giriRicevuti".

class Rpm
{
    public void CalcolaRPM(string giriRicevuti, out long RPM)
    {
        Stopwatch stopWatch = new Stopwatch();
        stopWatch.Start();

        if (giriRicevuti == "1")
        {
            stopWatch.Stop();
        }
            long duration = stopWatch.ElapsedMilliseconds;

            RPM =(duration/1000);
    }
}

Your method starts a stopwatch and then immediately measures how long the stopwatch has been running -- which is obviously not for a very long time, because you just started the stopwatch. — dtb, May 25 '13 at 14:42
@dtb, should stop when I get the next "1". That is what I am trying to achieve. I have put there a condition to stop. But obviously isn't working. — FeliceM, May 25 '13 at 14:44
@BernhardPoiss every time I get a message from serial port containing "1". — FeliceM, May 25 '13 at 14:45
use Equals method to compare. string.http://stackoverflow.com/questions/1659097/c-string-equals-vs — Alpesh Gediya, May 25 '13 at 14:50
The scope of `stopWatch` is the `CalcolaRPM` method. It isn't even static. You need to rearrange things so that it persists from one call to the next. — HABO, May 25 '13 at 14:56

Matthew Watson · Answer 1 · 2013-05-25T15:56:32.630

2

You will need to keep a stopwatch around outside the CalcolaRPM() method if you want to time things inbetween calls to it.

The easiest thing to do is to add it as a private field in the class.

Another problem is that when giriRicevuti isn't "1" you will need to return the last known RPM - we can solve that by keeping the last known RPM in a private field too.

And yet another issue is that the very first time the RPM is calculated, it cannot be accurate because there is no preceeding time to compare it with. We will solve this by returning -1 until we have a correct time to report.

And next, you are calculating the elapsed RPM as an integer calculation. Now imagine if things were slightly out so the elapsed time was always 999 milliseconds. Only a millisecond out, but your calculation of RPM = 999/1000 would result in zero.

You have several options, but the most likely are:

Return a double instead.
Round the value to the nearest RPM.

I went for the rounding. The RPM calculation was incorrect, so I'm correcting that at the same time:

lastRPM = (int) Math.Round(60000.0/((int) stopWatch.ElapsedMilliseconds));

Putting that altogether, here's a compilable test program (console app):

using System;
using System.Diagnostics;
using System.Collections.Generic;
using System.Threading;

namespace Demo
{
    class Rpm
    {
        private Stopwatch stopWatch = new Stopwatch();
        private int lastRPM = -1;

        // RPM will be -1 until we have received two "1"s       

        public int CalcolaRPM(string giriRicevuti)
        {
            if (giriRicevuti == "1")
            {
                if (stopWatch.IsRunning)
                    lastRPM = (int) Math.Round(60000.0/((int) stopWatch.ElapsedMilliseconds));

                stopWatch.Restart();
            }

            return lastRPM;
        }
    }

    class Program
    {
        void run()
        {
            test(900);
            test(1000);
            test(1100);
            test(500);
            test(200);
        }

        void test(int interval)
        {
            Rpm rpm = new Rpm();

            for (int i = 0; i < 10; ++i)
            {
                Thread.Sleep(interval);
                rpm.CalcolaRPM("0");
                rpm.CalcolaRPM("1").Print();
                rpm.CalcolaRPM("2");
            }
        }

        static void Main()
        {
            new Program().run();
        }
    }

    static class DemoUtil
    {
        public static void Print(this object self)
        {
            Console.WriteLine(self);
        }

        public static void Print(this string self)
        {
            Console.WriteLine(self);
        }

        public static void Print<T>(this IEnumerable<T> self)
        {
            foreach (var item in self) Console.WriteLine(item);
        }
    }
}

edited May 25 '13 at 15:56

answered May 25 '13 at 15:01

Matthew Watson

104,400
10
158
276

The initial calculation will tend to be inaccurate since the stopwatch was started at an arbitrary time. – HABO May 25 '13 at 15:03
I guess we should return -1 for the first calculation, since that will always be inaccurate. I'll update with that. – Matthew Watson May 25 '13 at 15:05
@MatthewWatson, thanks man but RPM is still 0 even if the string giriRicevuti gets update every 1 second. I have checked it with a break point. Moreover I have a filter on the incoming serial messages that detects the "1" and pass it to this method, so there are not other type of values passed to it. – FeliceM May 25 '13 at 15:14
@FeliceM I have one last issue to fix then, haha. :) I'm updating now, be a couple of mins – Matthew Watson May 25 '13 at 15:15
@HABO, I need to measure the RPM of a motor that is running at 5 to 10 turns per minute. Shouldn't be a problem. – FeliceM May 25 '13 at 15:15
@MatthewWatson, LOL, it is giving -1 now. But we are on the good direction. :-) – FeliceM May 25 '13 at 15:18
@FeliceM - Hang on. RPM is usually revolutions per minute. Elapsed milliseconds divided by 1000 (using integer arithmetic) isn't going to give you the right units. Don't you want `60000/elapsed ms`? – HABO May 25 '13 at 15:20
@HABO, lets try to get something out then we fix the math. – FeliceM May 25 '13 at 15:21
@HABO Yeah I think that needs sorted too :) – Matthew Watson May 25 '13 at 15:22
You can do away with `isFirstTime` by not starting the `stopWatch` when it is created. Use `stopWatch.IsRunning` to determine if it is the first pass. – HABO May 25 '13 at 15:27
@MatthewWatson, great job Mattew but keeps retuning always -1. I have also changed the timing, now the variable is updated every 1.5 seconds but still I get only -1. – FeliceM May 25 '13 at 15:31
@Hang on I'm going to have to actually compile a test program! – Matthew Watson May 25 '13 at 15:32
@MatthewWatson, no problem, really appreciated. It is few hours that I am going stupid with this. So I fully understand. – FeliceM May 25 '13 at 15:34
@MatthewWatson, I added a stopWatch.Start(); to get it going and now is working. Thanks for your support. I am getting very inaccurate readings but this is not your problem. I think is the elapsed time method. – FeliceM May 25 '13 at 16:14

score 0 · Accepted Answer · answered May 27 '13 at 12:43

Thanks to your comments and advices I ended up with this solution. I also simplified the method using a return and some floating variables to get more accuracy. This one is working for my app.

class Turns
{
    static DateTime prevTimeInstance = DateTime.Now;
    static float RPM = 0;

    public float Counts(int getTurn)
    {
        TimeSpan currentTimeSpan = TimeSpan.Zero;
        if (getTurn.Equals(1))
        {
            currentTimeSpan = DateTime.Now.Subtract(prevTimeInstance);
            prevTimeInstance = DateTime.Now;
            if (currentTimeSpan.TotalSeconds != 0)
                RPM = 60.0f / (float)currentTimeSpan.TotalSeconds;
        }
        return RPM;
    }
}

I would like to thank Mattew for the big help he gave me.

Elapsed time is not giving me expected result

2 Answers2