# Car computer in python / GPS tracking

I have an old car that I use for long distance driving and have coded an onboard computer based on a Raspberry Pi 3 and a few other modules. It's my first project in Python and while I've already gotten some help, I now have a version of my project which works quite well.

I use a FONA 808 from Adafruit as the cellular modem (via serial), the Sparkfun NEO-M9N as a GPS sensor (i2c), an OLED display (i2c) and a small temperature sensor via 1-wire. Here is a link to a picture of the computer in action, just so you have a better picture of it: https://www.instagram.com/p/CJO9HnNneg2/

I'd appreciate getting some optimization tips to make it run smoothly. I'm especially unsure on how I've used threading and if I'm being really efficient with the GPS and the logging etc... Thanks!

(also, if the question is missing some information, don't downvote, comment and I'll correct it)

import os
import glob
import serial
import subprocess
import urllib
import urllib.request
import urllib.parse
import array
import requests
from time import sleep
from luma.core.interface.serial import i2c
from luma.core.render import canvas
from luma.oled.device import sh1106
from PIL import ImageFont, Image, ImageDraw
import time
import board
import busio
import subprocess
import pymysql
import RPi.GPIO as GPIO
from haversine import haversine, Unit
import csv
import pandas
import datetime
import pathlib
import json
import smbus
import logging
import pynmea2

################## config display ##################
device.clear()
global pending_redraw
pending_redraw = False

### setup different fonts
FA_solid = ImageFont.truetype('/home/pi/Desktop/fonts/fa-solid-900.ttf', 12)
FA_solid_largest = ImageFont.truetype('/home/pi/Desktop/fonts/fa-solid-900.ttf', 40)
text_largest = ImageFont.truetype('/home/pi/Desktop/fonts/digital-7.ttf', 58)
text_medium = ImageFont.truetype('/home/pi/Desktop/fonts/digital-7.ttf', 24)
text_small = ImageFont.truetype('/home/pi/Desktop/fonts/digital-7.ttf', 18)

### Initialize drawing zone (aka entire screen)
output = Image.new("1", (128,64))

### coordinates always: padding-left, padding-top. the first pair of zone is always = start

# temp_ext
temp_zone = [(14,44), (36,64)]
temp_start = (14,44)
temp_icon_zone = [(0,48), (15,64)]
temp_icon_start = (3,48)

# alti
alti_zone = [(14,22), (69,40)]
alti_start = (14,22)
alti_icon_zone = [(0,24), (15,40)]
alti_icon_start = (0,26)

# distance
dist_zone = [(14,0), (69,21)]
dist_start = (14,0)
dist_icon_zone = [(0,4), (15,21)]
dist_icon_start = (0,4)

# speed
speed_zone = [(66,0), (128,45)]
speed_start = (66,0)

# GPRS status
gprs_zone = [(114,46), (128,64)]
gprs_start = (114,50)

# GPS status, incl. GPS startup icon
status_icon_zone = [(70,50), (88,64)]
status_icon_start = (70,50)
status_zone = [(86,46), (113,64)]
status_start_text = (86,46)
status_start = (86,50)

# usage
#device.display(output)

################## upload data from GPS folder via FONA to MySQL ##################
def fix_nulls(s):
for line in s:
yield line.replace('\0','')
while True:
sleep(20)
current_dir = "/home/pi/Desktop/data/gps"
archive_dir = "/home/pi/Desktop/data/gps/archive"
path, dirs, files = next(os.walk(current_dir))
file_count = len(files)
if file_count < 2:
print("Not enough GPS.csv files found so it's probably in use now or doesn't exist")
return
list_of_files = glob.glob(current_dir+"/*.csv")
oldest_file = min(list_of_files, key=os.path.getctime)
oldest_file_name = os.path.basename(oldest_file)

try:
global pending_redraw
pending_redraw = True
print("Opening remote db")

openPPPD()
print("Opening remote db: done")

db = pymysql.connect("XXX","XXX","XXX","XXX" )
cursor = db.cursor()
next(csv_data)
for row in csv_data:
if row:
cursor.execute('INSERT INTO gps_data_2 (gps_time, gps_lat, gps_long, gps_speed) VALUES (%s, %s, %s, %s)',row)
print("Commiting to db")
db.commit()
cursor.close()

closePPPD()

print("Successfully commited to db")
pending_redraw = True

os.rename(current_dir+"/"+oldest_file_name, archive_dir+"/archive_"+oldest_file_name)
sleep(60)

except Exception as e:
print("Database error:", e)
sleep(60)
return

sleep(300)

################## config and start GPS ##################
BUS = None
total_km = 0
prev_lat = 0
prev_long = 0

def connectBus():
global BUS
BUS = smbus.SMBus(1)

def parseResponse(gpsLine):
gpsChars = ''.join(chr(c) for c in gpsLine)
local_pending_redraw = False

if "$GNGGA" in gpsChars: if ",1," not in gpsChars: print("Looking for fix... (GGA)") add_to_image.rectangle(status_icon_zone, fill="black", outline = "black") add_to_image.rectangle(status_zone, fill="black", outline = "black") add_to_image.text(status_icon_start, "\uf124", font=FA_solid, fill="white") add_to_image.text(status_start, "\uf128", font=FA_solid, fill="white") local_pending_redraw = True return False try: nmea = pynmea2.parse(gpsChars, check=True) #print("GGA:", '%.6f'%(nmea.latitude), ",",'%.6f'%(nmea.longitude), ", sats:", nmea.num_sats, ", alt:", nmea.altitude) # GGA if "0.0" in str(nmea.latitude): return False if "0.0" in str(nmea.longitude): return False ## show fix + nb satellites #num_sats = str(nmea.num_sats) #num_sats = num_sats.lstrip("0") #add_to_image.rectangle(status_icon_zone, fill="black", outline = "black") #add_to_image.rectangle(status_zone, fill="black", outline = "black") #add_to_image.text(status_icon_start, "\uf124", font=FA_solid, fill="white") #add_to_image.text(status_start_text, num_sats, font=text_medium, fill="white") ## update altitude add_to_image.text(alti_icon_start, "\uf077", font=FA_solid, fill="white") add_to_image.rectangle(alti_zone, fill="black", outline = "black") add_to_image.text(alti_start, str('%.0f'%(nmea.altitude)), font=text_medium, fill="white") ## update total distance global reading_nr global total_km global prev_lat global prev_long dist = 0 if reading_nr != 1: dist = haversine(((float(prev_lat)), (float(prev_long))), ((float(nmea.latitude)), (float(nmea.longitude)))) total_km = total_km+dist add_to_image.text(dist_icon_start, "\uf1b9", font=FA_solid, fill="white") add_to_image.rectangle(dist_zone, fill="black", outline = "black") add_to_image.text(dist_start, "%0.1f" % total_km, font=text_medium, fill="white") prev_lat = nmea.latitude prev_long = nmea.longitude local_pending_redraw = True reading_nr +=1 except Exception as e: print("GGA parse error:", e) add_to_image.rectangle(status_zone, fill="black", outline = "black") local_pending_redraw = True pass if "$GNRMC" in gpsChars:
if ",A," not in gpsChars: # 1 for GGA, A for RMC
print("Looking for fix... (RMC)")
local_pending_redraw = True
return False
try:
nmea = pynmea2.parse(gpsChars, check=True)
if "0.0" in str(nmea.latitude):
return False
if "0.0" in str(nmea.longitude):
return False

## update speed
local_pending_redraw = True

## log every 5th GPS coordinate in CSV file
t = datetime.datetime.combine(nmea.datestamp, nmea.timestamp).strftime("%s")
d = datetime.datetime.combine(nmea.datestamp, nmea.timestamp).strftime("%Y%m%d%H")
filename = '/home/pi/Desktop/data/gps/gps_' + d + '.csv'
with open(filename, 'a', newline='') as csvfile:
gps_writer = csv.writer(csvfile, delimiter=',', quotechar='|', quoting=csv.QUOTE_MINIMAL)
gps_writer.writerow([t, nmea.latitude, nmea.longitude, nmea.spd_over_grnd*1.852])

#print("RMC: speed is", nmea.spd_over_grnd*1.852) # RMC
#print("RMC nmea.longitude:", nmea.longitude)

except Exception as e:
print("RMC parse error:", e)
local_pending_redraw = True
pass

if local_pending_redraw == True:
global pending_redraw
pending_redraw = True

c = None
response = []
try:
while True: # Newline, or bad char.
global BUS
if c == 255:
return False
elif c == 10:
break
else:
response.append(c)
parseResponse(response)
except IOError:
time.sleep(0.5)
connectBus()

connectBus()
while True:

################## config external thermometer ##################
def update_temp_ext(temp_signature='t=', update_interval=60):
sleep(10)
while True:
f = open('/sys/bus/w1/devices/28-012032ffbd96/w1_slave', 'r')
f.close()
equals_pos = lines[1].find(temp_signature)
if equals_pos != -1:
temp_string = lines[1][equals_pos+2:]
temp_c = round(float(temp_string) / 1000.0)
global pending_redraw
pending_redraw = True

#filename = 'data/temp_ext/tempext_' + datetime.datetime.now().strftime("%Y%m%d") + '.csv'
#with open(filename, 'a', newline='') as csvfile:
#    temp_ext_writer = csv.writer(csvfile, delimiter=' ', quotechar='|', quoting=csv.QUOTE_MINIMAL)
#    temp_ext_writer.writerow([str(temp_c)])

time.sleep(update_interval)

################## update display ##################
def update_display():
while True:
# there is a potential race condition here, not critical
global pending_redraw
if pending_redraw:
pending_redraw = False
device.display(output)
time.sleep(0.1)

################## start cellular connection ##################
def openPPPD():
print("Opening PPPD")

subprocess.call("sudo pon fona", shell=True)
print("FONA on")

global pending_redraw
pending_redraw = True

sleep(20)
try:
pending_redraw = True
print("Connection is on")
pending_redraw = True
return True
except:
print("No connection")
pending_redraw = True
sleep(5)
return False

# Stop PPPD
def closePPPD():
print("turning off PPPD")
subprocess.call("sudo poff fona", shell=True)
print("turned off")
return True

################## threading and program execution ##################
if __name__ == '__main__':


• Welcome to the Code Review Community where we review working code and provide suggestions for how to improve that code. The question is fairly good, but it is border line on working code. What makes it questionable is but is not super super reliable.. This is why I haven't voted it up. – pacmaninbw Jan 3 at 12:01
• @pacmaninbw thx for your feedback! I meant to say something like "it works, but I wouldn't trust it with my life". It's working at the moment, I believe the only hiccups are when there's a loose contact on my breadboard - I have ordered a PCB to remedy that – Damien Bourdonneau Jan 3 at 14:18
• According to some people on this site, Duct tape or Duck tape fixes everything including breadboards. – pacmaninbw Jan 3 at 14:31
• While this isn't really about the code, I'm a little concerned about you directly connecting to a remote MySQL server, since that tends to be pretty insecure (your credentials are probably broadcasted to the nearest cell tower in plaintext unless you've set up TLS). I would definitely recommend using a HTTPS POST request instead, along with a PHP script on the server to commit the information to the DB upon receiving said request, possibly also with some authentication, even if HTTP basic auth. – user9123 Jan 3 at 21:11
• @DamienBourdonneau I see. With the prepare/commit issue, it should be fine since if it's formatted as JSON, the server needs the whole message to start working on it, so there won't be any malformed data, but you could also include a checksum to be sure. As for POST body size, it should be configurable on the server, assuming your host lets you do that. You may want to contact their support and ask, but it should be pretty big by default anyway. Good luck though if you do decide to try it! :) – user9123 Jan 5 at 19:24

There's A LOT going on in your code but I'll try to give you some hints and some suggestions regarding the overall structure/workflow of the code.

• you have really abused the use of globals in your code. Some of them don't even make sense. I'd suggest you try and understand what the use of it is and stop using it unless really needed. For example, the use of global keyword outside a function has no effect (e.g. pending_redraw).
• for setting your fonts, you could easily create a function and a dataclass to stick to the DRY principle:
FONTS_BASEDIR = '/home/pi/Desktop/fonts/'

def setup_font(font_filename, value):
return ImageFont.truetype(
os.path.join(FONTS_BASEDIR, font_filename),
value
)

@dataclass
class Fonts:
fa_solid = setup_font('fa-solid-900.ttf', 12)
fa_solid_largest = setup_font('fa-solid-900.ttf', 40)
text_largest = setup_font('digital-7.ttf', 58)
text_medium = setup_font('fa-solid-900.ttf', 24)
text_small = setup_font('fa-solid-900.ttf', 18)

• remove the unused imports and try not to duplicate them.
• try to be as clear/accurate as possible in your comments. If I were to believe this: coordinates always: padding-left, padding-top. the first pair of zone is always = start which doesn't seem to be the case for temp_icon_zone and temp_icon_start ((0, 48) != (3, 48)).
• you have a lot of constants which look more like configuration variables. I'd suggest you create a config file to have easier access to all of these. Config variables should also be named using the UPPER_CASE notation. (e.g. temp_zone would be TEMP_ZONE = [(14, 44), (36, 64)] and temp_start would become: TEMP_START = TEMP_ZONE[0])
• try using os.path.join() instead of current_dir + "/" + oldest_file_name, archive_dir + "/archive_" + oldest_file_name when joining together pieces of a path. (see my example from setup_font function)
• you misspelled committed and committing
• here: db = pymysql.connect("XXX", "XXX", "XXX", "XXX") it looks like you're using clear-text credentials. That's bad practice and I'd suggest you stop doing that! There are multiple other ways of importing your credentials, one of which would be setting the password as an ENV Variable.
• this:
if "0.0" in str(nmea.latitude):
return False
if "0.0" in str(nmea.longitude):
return False


can be rewritten as this:

if "0.0" in str(nmea.latitude) or "0.0" in str(nmea.longitude):
return False


or even better:

return "0.0" not in str(nmea.latitude) or "0.0" not in str(nmea.longitude)

• here:
f = open('/sys/bus/w1/devices/28-012032ffbd96/w1_slave', 'r')
f.close()
equals_pos = lines[1].find(temp_signature)


you only seem to use whatever it's on the second line of that file, so why keep the whole file into memory? Try to break after you reached that very line:

with open('/sys/bus/w1/devices/28-012032ffbd96/w1_slave') as f:
for i, line in enumerate(f):
if i == 1:
equals_pos = line.find(...)
break

• that's great, thanks for the feedback! I'll look into your comments (which all make sense from a first read), implement them and edit/comment when confused or successful! Also, I'm not not sure about threading at the end and on which thread I join - does that look right to you? – Damien Bourdonneau Jan 3 at 14:24
• Please don’t edit your code but rather ask a follow-up question instead with the new code to be reviewed. Also, it’s worth allowing 1-2 days until accepting an answer since there might be valuable input still coming from other reviewers – Grajdeanu Alex Jan 3 at 14:27
• return "0.0" not in str(nmea.latitude) or "0.0" not in str(nmea.longitude) does not have the same behavior as the first rewrite because it changes a conditional return to an unconditional return – Setris Jan 3 at 17:55
• @GrajdeanuAlex I'm implementing everything atm but I have a few questions: following your advice, I now have an .ini file with different config variables etc. Is that a safe(r) place to store the mysql info? Also, regarding TEMP_ZONE = [(14, 44), (36, 64)]: I have added that to my ini file too but it seems that when I try to use it, it is not seen as a tuple but just some random string and thus the coordinates are not read correctly. Got any tips on that? Have been unsuccessful so far. The rest I've implemented as far as possible! – Damien Bourdonneau Jan 5 at 18:52
• "Is that a safe(r) place to store the mysql info?" -> not quite / it depends. It's usually a common practice to have a .env file where you can store these kind of things. You also have to make sure you don't add this .env file to git. The parsed configuration (from this env file) is used as a basis for adding environment variables. If you want to keep the data structures and use them in your code, you can use, at any point, any other data type format like a yaml file / json file or even a config.py. – Grajdeanu Alex Jan 5 at 20:06

Other minor points:

## Generators

for line in s:
yield line.replace('\0','')


can be simplified to

return (line.replace('\0', '') for line in s)


To see what the difference is, we borrow from some techniques in a different answer:

>>> def f():
...     for c in 'a b':
...         yield c.replace(' ', '')

>>> def g():
...     return (c.replace(' ', '') for c in 'a b')

>>> from inspect import isgeneratorfunction
>>> from dis import dis

>>> isgeneratorfunction(f)
True
>>> isgeneratorfunction(g)
False

>>> dis(f)
2           0 LOAD_CONST               1 ('a b')
2 GET_ITER
>>    4 FOR_ITER                18 (to 24)
6 STORE_FAST               0 (c)

16 CALL_METHOD              2
18 YIELD_VALUE
20 POP_TOP
22 JUMP_ABSOLUTE            4
26 RETURN_VALUE

>>> dis(g)
2           0 LOAD_CONST               1 (<code object <genexpr> at 0x00000236289E7F50, file "<stdin>", line 2>)
4 MAKE_FUNCTION            0
8 GET_ITER
10 CALL_FUNCTION            1
12 RETURN_VALUE

Disassembly of <code object <genexpr> at 0x00000236289E7F50, file "<stdin>", line 2>:
>>    2 FOR_ITER                18 (to 22)
4 STORE_FAST               1 (c)
14 CALL_METHOD              2
16 YIELD_VALUE
18 POP_TOP
20 JUMP_ABSOLUTE            2
24 RETURN_VALUE


This demonstrates that writing a generator function does not produce equivalent bytecode to writing a function that returns a generator. The generator function produces bytecode that is shorter, but the generator-returning function has code that's more terse; so in the end it's somewhat of a wash.

## Paths

It's not a good idea to bake /home/pi into your data directory paths. This should be configurable, and/or auto-discovered based on the location of the source. Beyond that, storing your application in a subdirectory of a login user's desktop directory is not a good idea. Do some research on the standard Unix directory layout, which would see your fonts in /usr/share/my_app, your application in /usr/bin, etc.

• How is converting from a generator to a generator expression any simpler? I don't see how moving the for loop from the line above to behind the expression could be called "simplified" when all you've done is rearrange the code. – Peilonrayz Jan 3 at 18:29
• @Peilonrayz I don't understand your comment. Knowing how the interpreter differentiates between these two styles is educational. – Reinderien Jan 3 at 18:58
• My original comment was actually just wrong, due to a typo in the example function. The yield style produces simpler bytecode, but the return style requires fewer lines of code. In the end it's a judgement call based on context. – Reinderien Jan 3 at 19:03
• @Reinderien is yield from best of both worlds? – Gavin S. Yancey Jan 4 at 6:08
• Autodiscovery isn't strictly necessary if you choose sane paths for your data. You can omit getcwd and hard-code a base path for your fonts like /usr/share/carcomp/fonts. – Reinderien Jan 5 at 19:20