Skip to main content
Code Review

Return to Answer

Bounty Awarded with 25 reputation awarded by Community Bot
added 67 characters in body
Source Link
Reinderien
  • 71k
  • 5
  • 76
  • 256

is a bad home for an application. You should take this opportunity to learn how standard Unix daemonsapplications store their data. Configuration in /etc, application script in /usr/bin, data files in /usr/share, etc. Learn about systemd.

To reinforce this: pi is a login user, and your device has internet connectivity. RunningI'm unclear on how your application is started and whether you intend on running it as a service, but whatever the case, you would benefit from making a jail that has this application run as a loginheavily permissions-restricted, dedicated-purpose user is a perfect recipe for a security disaster. I don't want your car to be hacked and I'm sure you don't either.

is a bad home for an application. You should take this opportunity to learn how standard Unix daemons store their data. Configuration in /etc, application script in /usr/bin, data files in /usr/share, etc. Learn about systemd.

To reinforce this: pi is a login user, and your device has internet connectivity. Running a service as a login user is a perfect recipe for a security disaster. I don't want your car to be hacked and I'm sure you don't either.

is a bad home for an application. You should take this opportunity to learn how standard Unix applications store their data. Configuration in /etc, application script in /usr/bin, data files in /usr/share, etc.

pi is a login user, and your device has internet connectivity. I'm unclear on how your application is started and whether you intend on running it as a service, but whatever the case, you would benefit from making a jail that has this application run as a heavily permissions-restricted, dedicated-purpose user.

Source Link
Reinderien
  • 71k
  • 5
  • 76
  • 256

First, I need to say that this is a really cool project; quite fun.

You ask:

Other than consistency, would you see any advantages in using python?

Bad code can be written in any language, but it's really easy to write bad code in PHP. At the risk of starting a language-religion flame war, there is not a single application I could in good conscience recommend to a new PHP build for which other languages aren't a better fit. Fun fact: according to Wikipedia,

72% of PHP websites use discontinued versions of PHP.

As such, many of the greyed-out entries in PHP Sadness still have significant impact. So less "use Python instead" and more "use something that isn't PHP". Anyway, on to less controversial topics:

Global code

Move lines like

device = sh1106(i2c(port=1, address=config['display']['i2c_port']), rotate=0)
device.clear()
pending_redraw = False
output = Image.new("1", (128,64))
add_to_image = ImageDraw.Draw(output)
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('digital-7.ttf', 24)
text_small = setup_font('digital-7.ttf', 18)
icons = { #to look up the icons on FontAwesome.com, remove quote marks and \u from the search query 
 "save": "\uf56f","cloud": "\uf0c2","check": "\uf058","upload": "\uf382","no_conn": "\uf127","location": "\uf124","question": "\uf128","altitude": "\uf077","distance": "\uf1b9","temperature": "\uf2c9" }

out of global scope. Among a long list of other reasons, this harms testability and pollutes your namespace. Also, since this is an embedded environment, I'd be worried about memory usage being impacted by long-lived references that needn't be.

Consider making classes, and/or moving code into free functions, and/or moving those classes and functions into more narrow-purpose Python module files.

Style

Get a linter. My favourite is PyCharm. It will help you learn the PEP8 standard, including (significantly) untangling your function definitions by separating them with two empty new lines.

Pathlib

These:

 current_dir = os.path.join(config['general']['folder'],config['general']['folder_data'])
 archive_dir = os.path.join(config['general']['folder'],config['general']['folder_data_archive'])

will be simplified by the use of pathlib. Store a Path for your config['general']['folder'] so that you don't need to re-traverse that configuration object. Drop your os.path calls.

Application location

/home/pi/

is a bad home for an application. You should take this opportunity to learn how standard Unix daemons store their data. Configuration in /etc, application script in /usr/bin, data files in /usr/share, etc. Learn about systemd.

To reinforce this: pi is a login user, and your device has internet connectivity. Running a service as a login user is a perfect recipe for a security disaster. I don't want your car to be hacked and I'm sure you don't either.

Method complexity

Break up upload_data into multiple subroutines, given its length and complexity.

Connection management

Surround your unprotected connect/close:

 db = pymysql.connect(config['db']['db_host'],config['db']['db_user'],config['db']['db_pw'],config['db']['db_name'])
 cursor = db.cursor()
 cursor.close()

in a try/finally. I would stop short of using a with statement since that library has surprising behaviour on __exit__.

Data vs. config

These are good examples of config entries:

mode = server
ping = https://XXX
db_host = XXX
db_user = XXX
db_pw = XXX
db_name = XXX

These are not:

TEMP_ZONE = [(14,44), (36,64)]
TEMP_START = (14,44)
TEMP_ICON_ZONE = [(0,48), (15,64)]
TEMP_ICON_START = (3,48)

So far as I can tell, those are UI coordinates. They aren't meaningful for a user to configure and should instead live in the application, perhaps in a dedicated UI constants file.

lang-py

AltStyle によって変換されたページ (->オリジナル) /