Batch - Pinging network IP's returns erroneous result in nested for loop iterations

Question

PLEASE bear in mind that I'm in the EARLY stages of learning Batch Scripting. I suspect it's not just a single bug in my script which is accountable for its strange behaviour. The test scenario is as follows. There is a file named netips.txt, containing a list of IP addresses on my LAN, to be pinged e.g.

192.168.1.1
192.168.1.2
192.168.1.3
192.168.1.33
192.168.1.34
192.168.1.35

When the script (bottom) is executed, the desired result is as follows: The user is prompted for a cycle interval and whether to receive a text file ipalert.txt, at the end of each cycle. Each IP address in netips.txt is then pinged, a pop-up msgbox for each IP which is Online ONLY should appear displaying the IP before disappearing after 3 seconds. A beep should accompany each pop-up. The whole process (cycle) should then be repeated according to the user interval choice.

What currently happens is (I'm not sure why), all IP's pop-up as Online along with a

' ' is not recognized as an internal or external command, operable program or batch file.

in the CMD window. The ipalert.txt log displays:

15/03/2015 at 12:44:23.12

192.168.1.1 Online
192.168.1.2 Online
192.168.1.3 Online
192.168.33 Online
192.168.34 Online
192.168.35 Online

Not all IP's should appear this way (some should be Offline). Can anyone please help me understand what is happening, and what I must do to correct this?

@echo off
setlocal enabledelayedexpansion

if exist c:\netips.txt goto START
echo.
echo Cannot find "C:\netips.txt"
echo.
echo Please create this file, containing a list of network IP addresses to monitor for activity
echo.
goto EOF

:START
echo.
echo [1] 5 mins
echo [2] 15 mins
echo [3] 30 mins
echo [4] 60 mins
echo.
set interval=0
choice /c 1234 /n /m "Please enter the activity notification rate:"
if errorlevel 1 set interval=300
if errorlevel 2 set interval=900
if errorlevel 3 set interval=1800
if errorlevel 4 set interval=3600

echo.
echo You will be notified every %interval% seconds

echo.
set log=0
choice /m "Would you like to automatically view the activity log after each cycle? "
if errorlevel 1 set log=Y
if errorlevel 2 set log=N

if "%log%"=="N" (
echo.
echo You can view the activity log [ipalert.txt] in the root directory
)

echo.
echo You can minimise this CMD window while it is running 
echo.
echo Press any key to begin
pause>nul

:LOOP
cls
echo.
echo Executing...

echo. >>c:\ipalert.txt
echo %date% at %time% >>c:\ipalert.txt
echo. >>c:\ipalert.txt

set state=down
for /f %%i in (c:\netips.txt) do (
for /f "tokens=5,7" %%a in ('ping -n 1 %%i') do (
if "x%%a"=="xReceived" if "x%%b"=="x1," set state=up
)

if !state!==up (
echo %%i Online >> c:\ipalert.txt
echo %%i Online > c:\ipnotify.txt
BELL rem special character for 'beep'
msg "%username%" /time:3 < c:\ipnotify.txt
del /q c:\ipnotify.txt
) else (
echo %%i Offline >> c:\ipalert.txt
)
set state=down
)

if "%log%"=="Y" (
start notepad c:\ipalert.txt
timeout /t 6 >nul
taskkill /im notepad.exe>nul
)

timeout /t !interval! /nobreak >nul
goto LOOP

endlocal

UPDATE

I've altered the LOOP sub (see hereunder) with some improvement, the correct IP's now pop-up as Online, however the ipalert.txt now ONLY displays those IP's, and not the Offline ones aswell...?

:LOOP
cls
echo.
echo Executing...

echo. >>c:\ipalert.txt
echo %date% at %time% >>c:\ipalert.txt
echo. >>c:\ipalert.txt

for /f %%i in (c:\netips.txt) do (
for /f "tokens=*" %%a in ('ping -n 1 %%i ^| find "TTL="') do (
if errorlevel 0 (
echo %%i Online >> c:\ipalert.txt
echo %%i Online > c:\ipnotify.txt
BELL
msg "%username%" /time:3 < c:\ipnotify.txt
del /q c:\ipnotify.txt
) 
if errorlevel 1 (
echo %%i Offline >> c:\ipalert.txt
)
)
)

Answer 1

Thanks for your (plural) help. I wish there was a way to mark more than one reply as the answer. Both JosefZ and MC ND addressed a number of very important faults with my script, and both were instructive and ultimately instrumental in my development of a working version. Below is my finished script (it appears to work for me as intended, without deviating too much from its original structure). Once again thanks all!

@echo off
title Network Activity Notifier
color 0a

cls
echo.
echo ============================
echo Network Activity Notifier 
echo ============================

setlocal enabledelayedexpansion

if exist c:\netips.txt goto START
echo.
echo Cannot find "C:\netips.txt"
echo.
echo Please create this file, containing a list of network IP addresses to monitor for activity
echo.
goto EOF

:START
echo.
echo [1] 5 mins
echo [2] 15 mins
echo [3] 30 mins
echo [4] 60 mins
echo.
set interval=0
choice /c 1234 /n /m "Please enter the activity notification rate:"
if errorlevel 1 set interval=300
if errorlevel 2 set interval=900
if errorlevel 3 set interval=1800
if errorlevel 4 set interval=3600

echo.
echo You will be notified every %interval% seconds

echo.
set log=0
choice /m "Would you like to automatically view the activity log after each cycle? "
if errorlevel 1 set log=Y
if errorlevel 2 set log=N

if "%log%"=="N" (
echo.
echo You can view the activity log [ipalert.txt] in the root directory
)

echo.
echo You can minimise this CMD window while it is running 
echo.
echo Press any key to begin
pause>nul

:LOOP
cls
echo.
echo Executing...

echo. >>c:\ipalert.txt
echo %date% at %time% >>c:\ipalert.txt
echo. >>c:\ipalert.txt

for /f %%i in (c:\netips.txt) do (
ping -n 1 %%i | find "TTL=" >nul
if errorlevel 1 (
echo %%i Offline >> c:\ipalert.txt
) else (
echo %%i Online >> c:\ipalert.txt
echo %%i Online > c:\ipnotify.txt
echo 
msg "%username%" /time:3 < c:\ipnotify.txt
del /q c:\ipnotify.txt
)
)

if "%log%"=="Y" (
start notepad c:\ipalert.txt
timeout /t 6 >nul
taskkill /im notepad.exe>nul
)

timeout /t !interval! /nobreak >nul
goto LOOP

endlocal

Answer 2

set "netIPs=c:\netips.txt"
set "ipAlert=c:\ipalert.txt"

:: ....

for /l %%a in (0) do (
    cls
    echo(
    echo Executing...

    >> "%ipAlert%" (
        setlocal enabledelayedexpansion
        echo(
        echo(!date! at !time!
        echo(
        endlocal
    )

    (for /f "usebackq" %%i in ("%netIPs%") do (
        ping -n 1 "%%~i" | find "TTL=" >nul && (
            echo %%i Online 
            msg "%userName%" /time:3 "%%i Online"
        ) || (
            echo %%i Offline 
        )
    )) >> "%ipAlert%"

    timeout /t %interval% /nobreak >nul
)

Three changes from your code.

1. If a value is used more than once (here the file references), it is better to use a variable

2. As there is no exit from the loop, the backwars goto has been replaced with an infinite for loop

3. Conditional executions is used to test the result of the ping operation.

Instead of

ping....
if errorlevel 1 (
    rem failure
) else (
    sucess 
)

the && (execute code if previous command has not raised errorlevel) and || (execute code if previous command has raised errorlevel) are used

Remember that "usually" errorlevel must be done from higher values to lower ones as the expresion if errorlevel n will be evaluated to true if errorlevel is equal or higher than n.

Answer 3

I don't intend to pick on your efforts, but it's much easier to do ping-type operations using vbscript or powershell than with batch. Same for reading/writing files and registry data. There's nothing wrong with batch, but it's just one tool and trying to do everything with one language is like pounding nails with a wrench just because the wrench worked great on a bolt.

Answer 4

Your script goes without any error message(s) for me but this kind of errors could be caused by wrong escaping special characters or by wrong number of () parentheses; to localize the erroneous place, force ECHO ON and do not clear the screen with cls temporarily.

However, ping -n 1 ... returns Received = 1, even if Destination host unreachable:

==>ping -n 1 -4 192.168.1.2

Pinging 192.168.1.2 with 32 bytes of data:
Reply from 192.168.1.100: Destination host unreachable.

Ping statistics for 192.168.1.2:
    Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),

I'd use next test with find and forced IPV4:

  set state=down
  for /f "tokens=*" %%a in ('ping -n 1 -4 %%i^|find "TTL="') do (
      set state=up
  )

I do not know what is your msg command. The following command in a batch file will trigger the default beep on most PCs: ECHO (^G). To type the BELL character (ASCII 7) use CtrlG or, holding Alt key, type 07 (numbers on the numeric keypad).