Problems with If then, elseif statement

[Aelyus]

HotKeySet("{ESC}", "Terminate")
While 1
   Sleep(10000)
    Do
        sleep(2000)
        send("h")
        sleep(2000)
     Until PixelGetColor (811, 512) = 0xFF6666
       sleep(2000)
       send("s")
       sleep(2000)
       send("h")
    If PixelGetColor (811, 512) = 0x33FF33 Then Sleep(2000)
       Send("d")
       Sleep(3000)
       Send("s")
   ElseIf PixelGetColor (811, 512) <> 0x33FF33 Then Do
       Sleep(5000)
       Send("s")
       Sleep(2000)
       Send("s")
       Sleep(2000)
       Send("h")
    Until PixelGetColor (811, 512) = 0x33FF33
       Sleep(1000)
       Send("d")
       Sleep(3000)
       Send("s")
EndIf
WEnd
Func Terminate()
   Exit 0
EndFunc

 

Code starts on line 1.

When checking syntax i get the "missing WEnd" error, when running the script in any other fashion or variation of if, then, else, it seems to miss the "If" "Then" portion and just goes to the "Elseif" portion.

 

I'm newer to autoit, and have been researching this problem for 2 solid days now. I have read everything off the first 5 pages of google when you search "Autoit if, then, else", dug through dozens of posts on stack exchange, This has to be something dumb i am missing or overlooking, could someone lend a second pair of eyes and tell me what i did wrong?

 

Thanks

[Jos]

There are multiple issues in your code and proper indentation helps to figure out what the issue is.
Use Tidy and au3check to help you with that.

Jos

pointer: line 13 and 17 are wrong and the statement behind the Then needs to be on a line of their own.

Edited July 23, 2016 by Jos

[Aelyus]

Ran Tidy on it and got this back

HotKeySet("{ESC}", "Terminate")
While 1
    Sleep(10000)
    Do
        sleep(2000)
        send("h")
        sleep(2000)
    Until PixelGetColor(811, 512) = 0xFF6666
    sleep(2000)
    send("s")
    sleep(2000)
    send("h")
    If PixelGetColor(811, 512) = 0x33FF33 Then Sleep(2000)
    Send("d")
    Sleep(3000)
    Send("s")
ElseIf PixelGetColor(811, 512) = 0xFF6666 Then
    Sleep(5000)
    Send("s")
    Sleep(2000)
    Send("s")
    Sleep(2000)
    Send("h")
Until PixelGetColor(811, 512) = 0x33FF33
EndIf
Sleep(1000)
Send("d")
Sleep(3000)
Send("s")
WEnd
Func Terminate()
    Exit 0
EndFunc   ;==>Terminate

I assume by au3check you mean the "SyntaxCheck Prod" under tools, as when i click the au3check .exe in programs it opens nothing. When running the syntax check all i get is the same error as before 2 errors both missing Wend,  I have tried everything i could find about loops, nested loops, etc, I've been at this for two days. I get that as a coder, you would rather "show a guy how to fix it himself" vs. tell him what he did wrong, but after two days, two solid days i cannot find any information that leads me to understand what is wrong here. I have done several other autoit projects and always been able to find the answers myself. Just this once, could you make an exception and give me a straight forward "this is what you did wrong" answer that i can learn from? I am not asking you to fix my code and give me a copy/paste solution, just for you to say "hey, you misused X, or forgot to X, or missed X. 

[kcvinu]

expandcollapse popup

HotKeySet("{ESC}", "Terminate")

While 1
    Sleep(10000)

    Do
        sleep(2000)
        send("h")
        sleep(2000)
    Until PixelGetColor(811, 512) = 0xFF6666

    sleep(2000)
    send("s")
    sleep(2000)
    send("h")

    If PixelGetColor(811, 512) = 0x33FF33 Then 
        Sleep(2000)
        Send("d")
        Sleep(3000)
        Send("s")
    ElseIf PixelGetColor(811, 512) = 0xFF6666 Then
        Sleep(5000)
        Send("s")
        Sleep(2000)
        Send("s")
        Sleep(2000)
        Send("h")
    
;Until PixelGetColor(811, 512) = 0x33FF33  - ; There is no Do for this loop. So i am commenting this out.
    EndIf
    
    Sleep(1000)
    Send("d")
    Sleep(3000)
    Send("s")
WEnd

Func Terminate()
    Exit 0
EndFunc   ;==>Terminate

I just tried to indent your code and this is the result. 

[Aelyus]

Thanks KCVINU, can you tell me the process where you got that result, i opened Tidy, selected the file i wanted to "tidy" and what i pasted above was what it gave me, is there some parameters that need to be set that i missed?

[kcvinu]

I did it manually.  First of all, if you want us to help you, then please state what do you want to do ?

[FlashpointBlack]

For future reference, you can indeed have a single-line if-then statement, but only if you have one single operation to perform.

Example, Note no ending "Endif":

$Today = "Saturday"

if $Today = "Saturday" Then MsgBox(0,"Woohoo!","It's Saturday, baby!")

If you have multiple operations to perform on a condition, you can't include anything after "Then" in your statement, and it must be on the following line:

$Today = "Saturday"

if $Today = "Saturday" Then
    MsgBox(0,"Woohoo!","It's Saturday, baby!")
    MsgBox(0,"Party!","Party at my house!")
EndIf

Proper indentation is really essential to being able to read your code. Being set up as it is in my example, or in kcvinu's cleaned code, it's easy to see what is going to happen given conditions. The reason tidy didn't work properly is because your code was incorrect from the start.

Edited July 23, 2016 by FlashpointBlack
Additional snippet of information

[SadBunny]

Also, about that tidy thing, just run it from the full version of scite (Tools -> Tidy AutoIt Source, hotkey is Ctrl+T) and you should see something like the below, and it is pretty descriptive about what's wrong with which lines:

[FlashpointBlack]

Just now, SadBunny said:

Also, about that tidy thing, just run it from the full version of scite (Tools -> Tidy AutoIt Source, hotkey is Ctrl+T) and you should see something like the below, and it is pretty descriptive about what's wrong with which lines

Nice tip! I never use tidy since I use some custom formatting, so this is good to know.

[Aelyus]

4 minutes ago, kcvinu said:

I did it manually.  First of all, if you want us to help you, then please state what do you want to do ?

Sorry, i should have started with that.

My mom spends countless hours after work going through invoices for her business to see which are paid and which are not.  This script is a sort of macro to do it for her automatically. They are scanned image files, the paid ones have a green box on them, the unpaid have a red box on them.

 

The script is basically searching for green and red boxes and saving the green ones using the keyboard shortcuts that the software she uses has predefined.

In non machine speak

Hit the "h" key until it finds an unpaid invoice

Then hit "s" to save it, then proceed to the "h" key to move to the next one.

If the next one is green as well, then "d" to mark it as paid, then "s" to save it

If it is not green, then "s", "s" to save it unpaid, then "h" to move to the next one until it finds the next green one,

then "d" to flag it, and "s" to save it

then repeat the loop.

 

I'm quite sure i messed it up, but i tried adding the DO that was missing and ended up with this code

 

expandcollapse popup

HotKeySet("{ESC}", "Terminate")

While 1
    Sleep(10000)

    Do
        sleep(2000)
        send("h")
        sleep(2000)
    Until PixelGetColor(811, 512) = 0xFF6666

    sleep(2000)
    send("s")
    sleep(2000)
    send("h")

    If PixelGetColor(811, 512) = 0x33FF33 Then
        Sleep(2000)
        Send("d")
        Sleep(3000)
        Send("s")
    ElseIf PixelGetColor(811, 512) = 0xFF6666 Then

    Do
       Sleep(5000)
        Send("s")
        Sleep(2000)
        Send("s")
        Sleep(2000)
        Send("h")
    Until PixelGetColor(811, 512) = 0x33FF33
    EndIf

    Sleep(1000)
    Send("d")
    Sleep(3000)
    Send("s")
WEnd

Func Terminate()
    Exit 0
EndFunc   ;==>Terminate

It's still wrong, i think its because i need to nest some loops, but i am not sure as i cannot find too much info on it.

[SadBunny]

So you expect green or red boxes in many scanned images to always be exactly 0xFF6666 or 0x33FF33? Odds of that being exactly the case in every scanned image are 0.

[Aelyus]

1 minute ago, SadBunny said:

So you expect green or red boxes in many scanned images to always be exactly 0xFF6666 or 0x33FF33? Odds of that being exactly the case in every scanned image are 0.

They are, i checked over 1,000 of them with a pixel color checker tool i made with autoit and every one of them were those two colors

[kcvinu]

As far as i understand your problem, this is the pseudocode i can give you.

Inside a loop

1. Set an initial sleep time. (say 2 secs)

2. Check the pixel color in your desired location.

3. Check it with an if statement

    a. If it is green

          do the things for a green box

     b If it is red

          do the things for a red box

4 Repeat the loop. 

 

 

[Aelyus]

23 minutes ago, SadBunny said:

Also, about that tidy thing, just run it from the full version of scite (Tools -> Tidy AutoIt Source, hotkey is Ctrl+T) and you should see something like the below, and it is pretty descriptive about what's wrong with which lines:

 

 

Thanks Bunny, i noticed i was still in Lite, but switched to full and ran the command you stated on the code i pasted below, and i got 0 of the red error boxes like you show. I can see by what is showing in your post that i must need to nest loops to make this work? is that a correct assumption? Also, would you happen to know why when i Ctrl +T i do not get the notifications in red like you do?

 

expandcollapse popup

HotKeySet("{ESC}", "Terminate")

While 1
    Sleep(10000)

    Do
        Sleep(2000)
        Send("h")
        Sleep(2000)
    Until PixelGetColor(811, 512) = 0xFF6666

    Sleep(2000)
    Send("s")
    Sleep(2000)
    Send("h")

    If PixelGetColor(811, 512) = 0x33FF33 Then
        Sleep(3000)
        Send("d")
        Sleep(3000)
        Send("s")
    ElseIf PixelGetColor(811, 512) = 0xFF6666 Then

        Do
            Sleep(5000)
            Send("s")
            Sleep(2000)
            Send("s")
            Sleep(2000)
            Send("h")
        Until PixelGetColor(811, 512) = 0x33FF33
    EndIf

    Sleep(1000)
    Send("d")
    Sleep(3000)
    Send("s")
WEnd

Func Terminate()
    Exit 0
EndFunc   ;==>Terminate

 

[SadBunny]

Because the code you are working with now is not broken any more Remove the "EndIf" and try again, you should see one of those red errors.

[SadBunny]

Ok, now that I helped a bit, I must say that your code is absolutely not going to work for your goal.

1. Even if you scan the exact same document a 1000 times without touching the scanner, it's not always going to be exactly the same color on exactly the same pixel with exactly that crisp distribution of RGB every time, and certainly not with R or G be exactly FF (the maximum value). That's simply impossible.
2. Those keys you're sending and those long delays don't seem to make any sense in the light of sorting financial administration by way of displaying all of the thousands of invoices.

Lastly, if this IS going to assist financial administration in some zany way, at the very least you should write some code in case the pixel is not recognised in one of those two colors is not found (but any other color) and present an error or something.

[Aelyus]

2 minutes ago, SadBunny said:

Ok, now that I helped a bit, I must say that your code is absolutely not going to work for your goal.

1. Even if you scan the exact same document a 1000 times without touching the scanner, it's not always going to be exactly the same color on exactly the same pixel with exactly that crisp distribution of RGB every time, and certainly not with R or G be exactly FF (the maximum value). That's simply impossible.
2. Those keys you're sending and those long delays don't seem to make any sense in the light of sorting financial administration by way of displaying all of the thousands of invoices.

Lastly, if this IS going to assist financial administration in some zany way, at the very least you should write some code in case the pixel is not recognised in one of those two colors is not found (but any other color) and present an error or something.

As i have stated, i ran over 1,000 test invoices through, and the same pixel in the same place was the same color, every time. It's a high end scanner with good alignment and "true color" according to what it says on the machine.

 

The code works, right up until after the if, then, elseif statement. Once if completes the elseif, it is suppose to move along down to the last bit of code after the endif

Do
            Sleep(5000)
            Send("s")
            Sleep(2000)
            Send("s")
            Sleep(2000)
            Send("h")
        Until PixelGetColor(811, 512) = 0x33FF33
    EndIf

    Sleep(1000)
    Send("d")
    Sleep(3000)
    Send("s")

Then restart the loop. Instead, it just keeps repeating the "s" "s" "h" portion as if the condition was not met and never goes to the "d" "s" to flag, save and reset the loop.

 

I have to get this much to work before i am going to try and add in more code in case the pixel is not recognized, or the dozen other things that could possibly go wrong along the way while running.

[SadBunny]

The only reason for that loop to keep going on forever is that apparently the (811, 512) pixel color never equals that 0x33FF33. Make sure you check the correct pixel, maybe do a mousemove to that pixel before you do the check just so you can make sure you are using the correct coordinates and stuff.

Also, do you know about AU3Info? (Ctrl + F6 from scite.) Insanely useful. The "Mouse" tab has a built-in color checker as well.

[kcvinu]

@Aelyus ,

Do you read my post ?. I think you don't need more than one loop. Just make sure that the image window is active and check the pixel color. Then process the result.

Edited July 23, 2016 by kcvinu

[Aelyus]

1 minute ago, kcvinu said:

@Aelyus ,

Do you read my post ?. I think you don't need noree than one loop. Just make sure that the image window is active and check the pixel color. Then process the result.

Oh, sorry, it got lost above all the other comments some how when i clicked the notification. There is for sure a looping issue here, and most likely going to need to figure out a nested loop, i am reaching my breaking point for the day with this, as i cannot seem to get many straight forward answers. I may have to just hang it up for the day, and come back to it later. This is what i get for trying to be helpful and give my mom a break. I should have thought more before i opened my mouth.