HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Get external IP via email

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
externalgetviaemail

Problem

I wrote a script for my Raspberry Pi that keeps checking my external IP. If it changes, the Raspberry Pi will send me the new IP via email.

Any suggestions for improvements?

#!/usr/bin/python3

import os, time

def getIP():
        os.system('dig +short myip.opendns.com @resolver1.opendns.com > raspip.txt')
        with open('raspip.txt', 'r') as f:
                return f.readline().rstrip('\n')

def sendIP(currentIP):
        os.system('sendEmail '                            +
                  '-o tls=yes '                           +
                  '-f \'***@outlook.com\' '               +
                  '-t \'***@outlook.com\' '               +
                  '-s \'smtp.live.com\' '                 +
                  '-xu \'***@outlook.com\' '              +
                  '-xp \'**password**\' '                 +
                  '-u \"raspberry via python\" -m '       +
                  '\"' + currentIP + '\"')

def main():
        lastIP = None
        while (True):
                currentIP = getIP()
                if (lastIP != currentIP):
                        sendIP(currentIP)
                        lastIP = currentIP
                time.sleep(60)

if __name__ == "__main__":
        main()

Solution

You should probably prefer #!/usr/bin/env python3 as it's less likely to vary on different systems. It's not a big deal, though.

You should use snake_case, even for IP, so names like get_ip and send_ip.

Don't call os.system; use subprocess.Popen instead:

subprocess.Popen('dig +short myip.opendns.com @resolver1.opendns.com > raspip.txt'.split())


subprocess.Popen([
    'sendEmail',
    '-o', 'tls=yes',
    '-f', '***@outlook.com',
    '-t', '***@outlook.com',
    '-s', 'smtp.live.com',
    '-xu', '***@outlook.com',
    '-xp', '**password**',
    '-u', 'raspberry via python', '-m',
    current_ip
])


This is safe against quoting errors (what if the IP contained "?).

You also didn't need the +s; strings automatically concatenate. You don't need to escape 's inside "" or " inside '' - but it doesn't matter with Popen since you don't need to worry about quoting.

You do:

with open('raspip.txt', 'r') as f:
        return f.readline().rstrip('\n')


It's more idiomatic to treat files as iterables, so instead one would do

with open('raspip.txt', 'r') as f:
        return next(f).rstrip('\n')


This also lets you add a default if wanted with next(f, default). It might be a better idea to catch the error explicitly, since StopIteration can interfere with generators:

with open('raspip.txt', 'r') as f:
        try:
                return next(f).rstrip('\n')
        except StopIteration:
                raise ValueError("No text in raspip.txt!")


You also don't need the 'r' argument - it's default.

Don't put brackets around conditions in loops of ifs. It's just while True: and if last_ip != current_ip.

A more fancy example might use, say, Python's email library - but this is fine for what it does.

Code Snippets

subprocess.Popen('dig +short myip.opendns.com @resolver1.opendns.com > raspip.txt'.split())
subprocess.Popen([
    'sendEmail',
    '-o', 'tls=yes',
    '-f', '***@outlook.com',
    '-t', '***@outlook.com',
    '-s', 'smtp.live.com',
    '-xu', '***@outlook.com',
    '-xp', '**password**',
    '-u', 'raspberry via python', '-m',
    current_ip
])
with open('raspip.txt', 'r') as f:
        return f.readline().rstrip('\n')
with open('raspip.txt', 'r') as f:
        return next(f).rstrip('\n')
with open('raspip.txt', 'r') as f:
        try:
                return next(f).rstrip('\n')
        except StopIteration:
                raise ValueError("No text in raspip.txt!")

Context

StackExchange Code Review Q#85298, answer score: 5

Revisions (0)

No revisions yet.