[RESOLVED] serial port program hogging cpu

[atanq]

Hi,

i have the following code which runs ok, but it hogs upto 95% of the CPU. am i doing something really silly here?

program outline: a timer tick event = 1000ms scans through 30 readlines of data on the comport for a specific line of hex, and then extracts information from the next line and converts it to decimal for viewing on the screen.

i've only included the main part of the code to try and keep it simple. thanks in advance,

aaron.

Public Class Form1
    Private WithEvents serial As New IO.Ports.SerialPort
    Public grab As String
    Public cardIndex As Integer
    Private Sub Form1_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
        openPort()
        Timer1.Enabled = True
    End Sub
    Public Sub openPort() ' Open Serial Port and initialize.
        ' Timer1.Enabled = False
        If serial.IsOpen = False Then
            serial.BaudRate = 9600
            serial.PortName = "COM4"
            serial.StopBits = 1
            serial.Parity = IO.Ports.Parity.None
            serial.DataBits = 8
            serial.WriteTimeout = 2000
            serial.ReadTimeout = 2000 'Set timeout to 2 seconds.
            Try
                serial.Open()
                System.Threading.Thread.Sleep(50)
            Catch ex As Exception
                MsgBox(ex.Message)
            End Try
        End If
        If serial.IsOpen Then
            System.Threading.Thread.Sleep((11000 / serial.BaudRate) + 2) ' Min. 11 bit delay (startbit, 8 data bits, parity bit, stopbit)
        End If
    End Sub
    Private Sub Timer1_Tick(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Timer1.Tick
        searchData()
    End Sub
    Private Sub searchData()
        Timer1.Enabled = False
        For runThrough As Integer = 0 To 30
            Dim test As String
            Dim i As Integer
            Dim DataIn As String
            DataIn = ""
            If serial.IsOpen = True Then
                If serial.BytesToRead > 0 Then
                    DataIn = serial.ReadLine
                End If
            End If
            grab = ""
            For i = 1 To DataIn.Length - 1 ' This removes the first null character from the read line.
                test = DataIn.Chars(i)
                grab = grab + test
            Next
            If grab = "+010100B3" Then cardIndex = 1 : GetCardValues() ' card 1 info found. go and sort out data.
            Timer1.Enabled = True
        Next
    End Sub
    Public Sub GetCardValues() ' Sort data out for current card.
        Dim i As Integer
        Dim test As String
        Dim temp As String
        Dim check As String
        check = ""
        Dim DataIn As String
        DataIn = ""
        temp = ""
        If serial.BytesToRead > 0 Then
            grab = serial.ReadLine
        End If
        For i = 1 To grab.Length - 1 ' This removes the first null character from the read line again.
            test = grab.Chars(i)
            temp = temp + test
        Next
        For a As Integer = 0 To 6
            check = check + temp.Chars(a) ' look for a values identifier.
        Next
        If check = "+FD1803" Then
            grab = temp
            temp = ""
            For a As Integer = 7 To 10
                temp = temp + grab.Chars(a)
            Next
            temp = Convert.ToInt32(temp, 16).ToString
            If temp <= 100 Then
                If cardIndex = 1 Then Box1.Text = " " + temp + " LEL"
            End If
            temp = ""
        End If
    End Sub
    Private Sub Form1_FormClosing(ByVal sender As Object, ByVal e As System.Windows.Forms.FormClosingEventArgs) Handles Me.FormClosing
        closePort()
    End Sub
    Private Sub closePort()
        Try
            serial.Close()
            System.Threading.Thread.Sleep(50)
        Catch ex As Exception
            MsgBox(ex.Message)
        End Try
    End Sub
End Class
________________________________________

[Sitten Spynne]

At first I was suspicious the timer interval was too fast, but 1 second is hardly fast.

Since the timer tick calls searchData(), which then reads in a lot of characters and uses inefficient string concatenation to extract a portion of the data it read, then conditionally calls GetCardValues() which also does a lot of inefficient string concatenation, I could see the code using a lot of CPU. Try replacing your multiple "<some string> = <some string> + <some chars>" lines with usage of the System.Text.StringBuilder class; if you use that class properly you will cut down on memory allocations by a large amount.

In fact, you don't really need the for loops in many cases. This block:

For i = 1 To DataIn.Length - 1 ' This removes the first null character from the read line.
    test = DataIn.Chars(i)
    grab = grab + test
Next

... is equivalent to this:

grab = DataIn.Substring(1, DataIn.Length - 1)

That's bound to be more efficient.

Why? Strings are immutable arrays of characters. This means they can't be changed once created. When you concatenate a string to the end of another string, a new string is created and characters are copied from the old one. If you run your concatenation loop 10 times, 10 temporary strings are allocated and 10 + 9 + 8 + ... + 1 = 55 characters have to be copied. When you use Substring, only 1 string is created and you only copy the length of that string.

It's not a guarantee, but it might help.

[atanq]

thanks for the lightning reply! i'll give those suggestions a try, thanks very much. aaron

[Shaggy Hiker]

As a general rule, anything working with strings is going to be slower than working with numbers. Concatenation, in my experience, is likely to be particularly slow. Therefore, using Substring is really important in this case. However, it looks like your data is going to contain a certain amount of text info, which means that you will be using strings, like it or not.

Be sure you have Option Strict ON, as that generally increases efficiency. I have a feeling that the efficiency increase with Option Strict is greater for strings, but I suspect that is not really true.

[atanq]

Thanks, but how do i use option strict? It rings a bell but im not sure

[Shaggy Hiker]

You can set Option Strict in the Project Settings menu. That will set it for the entire program. You could also add it to the top of any file by typing Option Strict ON as the first line of the file, but why bother when you can do so for the whole program? It is certainly worth doing, as it teaches better habits, catches a few bugs that would otherwise slip through, but most importantly, in your case, you could see a performance boost of possibly as much as 30&#37;. That benefit depends largely on the code that you are running, but since you are seeing a noticeable slow down, you are more likely to see a performance benefit than most people.

[atanq]

this is where i've declared the option:

project>"projectname"properties>

name: setting
type: string
scope: user
value: option strict on

is this correct?
thanks,

[Shaggy Hiker]

That looks like a user setting.

On the Project menu, select the project properties. Then, on the Compile tab, check the checkbox for Option strict.

[atanq]

thanks, found it. but i don't have access to the pc thats running the program until i get back into work on monday, so i'll let you know what i find. cheers for all the assistance.

[atanq]

finally fixed it! i tried all the suggestions for changing the code, and none helped, so i tried it on a different pc with a local com port, as opposed to a usb-serial com port and it worked fine.

some digging around on the net, i found that this was a known issue with vb and prolific adapters. all i had to do was to install the latest version of the driver and hey presto!

thanks for all the help guys,

aaron.