Multithreading or something different

Question

This is the first time I face a problem like this. Not being this my profession but only my hobby, I have no previous references. In my program I have added one by one several functions to control a machine. After I added the last function (temperature measurement), I have started experiencing problems on other functions (approx. 8 of them running all together. The problem I am experiencing is on a chart (RPM of a motor) that is not related to this function but is affected by it. You see the difference between these two charts with and without the temperature measurement running. The real speed of the motor is the same in both charts but in the second one I loose pieces on the fly because the application slows down.

Without the temperature function. With the temperature function

Particularly this function is disturbing the above control and I think is because the work load is becoming heavy for the application and or because I need sampling so there is some time waiting to get them:

private void AddT(decimal valueTemp)
    {
        sumTemp += valueTemp;
        countTemp += 1;
        if (countTemp >= 20) //take 20 samples and make average
        {
            OnAvarerageChangedTemp(sumTemp / countTemp);
            sumTemp = 0;
            countTemp = 0;
        }
    }
    private void OnAvarerageChangedTemp(decimal avTemp)
    {
        float val3 = (float)avTemp;
        decimal alarm = avTemp;


            textBox2.Text = avTemp.ToString("F");


           if (alarm > 230)
           {

               System.Media.SoundPlayer player = new System.Media.SoundPlayer();
               player.Stream = Properties.Resources.alarma;
               player.Play();
               timer4.Start();
           }

           else
           {
               timer4.Stop();
               panel2.BackColor = SystemColors.Control;
           }
    }

I am wondering if running this function on a different thread would solve the problem and how I can do that? Or if there is a different way to solve the problem.Sample code will be appreciated.

Update, added method call.

This is how I call the method AddT

if (b != "")
            {

                decimal convTemp; //corrente resistenza
                decimal.TryParse(b, out convTemp);
                AddT(convTemp);}

This is how I receive the data from the serial and pass it to the class that strips out unwonted chars and return values to the different variables. This is the class that strips out the unwonted chars and return the values. And this is how I manage the serial incoming data. Please do not laugh at me after seeing my coding. I do a different job and I am learning on my own.

Answer 1

It's very hard to tell if there's anything wrong and what it might be - it looks like subtle problem.

However, it might be easier to get a handle on these things if you refactor your code. There are many things in the code you've shown that make it harder than necessary to reason about what's happening.

• You're using float and decimal - float isn't that accurate but small and fast; decimal (tries) to be precise but especially is predictable since it rounds errors the way a human might in base-10 - but it is quite slow, and is usually intended for calculations where precise reproducibility is necessary (e.g. financial stuff). You should probably use double everywhere.
• You've got useless else {} code in the Stripper class.
• Your Stripper is an instatiable class, when it should simply be a static class with a static method - Stripper is stateless.
• You're catching exceptions just to rethrow them.
• You're using TryParse, and not checking for success. Normally you'd only use TryParse if you (a) expect parsing to fail sometimes, and (b) can deal with that parse failure. If you don't expect failure or can't deal with it, you're better off with a crash you learn about soon than a subtly incorrect values.
• In stripper, you're duplicating variables such as _currentMot, currentMot, and param4 but they're identical - use only the parameter, and give it a logical name.
• You're using out parameters. It's almost always a better idea to define a simple struct and return that instead - this also allows you to ensure you can't easily mix up variable names, and it's much easier to encapsulate and reuse functionality since you don't need to duplicate a long call and argument definition.
• Your string parsing logic is too fragile. You should probably avoid Replace entirely, and instead explicitly make a Substring without the characters you've checked for, and you have some oddly named things like test1 and test2 which refer to a lastChar that's not the last character - this might be OK, but better names can help keep things straight in your head too.
• You have incorrect code comments (decimal convTemp; //corrente resistenza). I usually avoid all purely technical code comments; it's better to use descriptive variable names which are another form of self-documenting code but one in which the compiler can at least check if you use them consistently.
• Rather that return 4 possibly empty values, your Stripper should probably accept a parameter "sink" object on which it can call AddT AddD and AddA directly.

I don't think any of the above will fix your issue, but I do believe they're help keep your code a little cleaner and (in the long run) make it easier to find the issues.

Answer 2

your problem is in the parsing of the values you have

decimal.TryParse(a, out convRes);
AddA(convRes);

and don't check for failed values you only accept the value if it returns true

if(decimal.TryParse(a, out convRes))
{
   AddA(convRes);
}

you may have more errors but this one is making you process 0 values every time the TryParse fails.