3
\$\begingroup\$

I built this lamp for my school project, using the M5GO Kit.

The M5GO bot has 3 buttons:

  • the left button (state A) is used for advancing to the next date,
  • the middle button (state B) is for entering "study" mode where it keeps tracks of how long you are studying for the session. It also has an ultrasonic sensor that detects the distance between the lamp and the user, and a light sensor to detect how much light the user is getting. It then delivers how much light is needed to deliver to the user. It amins around 1500 Lux to the user and then uses the inverse square law to calculate how "strong" the RGB LED needs to be and set it to that level.
  • The right button (state C) is used to display statistics about your studying time.

My code is really hacky, and there are a lot of redundancies. Also, I ran into troubles: for example clicking different buttons in quick succession could cause the display to bug out. How can I refactor my code so that it is cleaner and avoid bugs?

In case my description is not good, you can take a look at the prototype.

from m5stack import *
from m5ui import *
from uiflow import *
from math import floor
import unit
import machine
import utime
import neopixel
import time

def set_state_a():
  global state
  state = 'A'
  set_all(0,0,0)
  time_label.hide()
  studying_label.hide()
  average_label.hide()
  sum_label.hide()
  max_label.hide()
  min_label.hide()
  date_label.show()
  welcoming_label1.show()
  welcoming_label2.show()
  set_all(0,0,0)

def set_state_b():
  global state
  state = 'B'
  date_label.hide()
  welcoming_label1.hide()
  welcoming_label2.hide()
  average_label.hide()
  sum_label.hide()
  max_label.hide()
  min_label.hide()
  studying_label.show()
  time_label.show()

def set_state_c():
  global state
  state = 'C'
  set_all(0,0,0)
  date_label.hide()
  welcoming_label1.hide()
  welcoming_label2.hide()
  studying_label.hide()
  time_label.hide()
  sum_label.show()
  average_label.show()
  max_label.show()
  min_label.show()
  
def set_all(r, g, b):
  for i in range(3):
    np[i] = (r, g, b)
  np.write()
  
def cal_date_str(day, month, year):
  day_text = str(day) if day >= 10 else '0' + str(day) 
  month_text = month_converter[month]
  year_text = str(year)
  date_label.setText(day_text+ ' ' + month_text + ' '+ year_text)
  
def cal_hr_str(start, label, seconds):
  second_round = round(seconds)
  min, sec = divmod(second_round, 60)
  hour, min = divmod(min, 60)
  sec_text = str(sec) if sec >= 10 else '0' + str(sec) 
  min_text = str(min) if min >= 10 else '0' + str(min) 
  hour_text = str(hour) if min >= 10 else '0' + str(hour) 
  label.setText(start + hour_text + ':' + min_text + ':'+ sec_text)

def buttonA_wasPressed():
  global state, day, month, year, study_seconds, study_array
  set_state_a()
  day += 1
  study_array.append(study_seconds/1000)
  study_seconds = 0

  if month == 2:
    is_leap = year % 4 == 0 and (year % 100 != 0 or year % 400 == 0)
    if (is_leap and day > 29) or (not is_leap and day > 28):
      day = 1
      month = 3
  elif month == 12 and day > 31:
    day = 1 
    month = 1 
    year += 1 
  elif month in (1, 3, 5, 7, 8, 10) and day > 31:
    day = 1 
    month += 1 
  elif month in (4, 6, 9, 11) and day > 30:
    day = 1 
    month += 1 
  cal_date_str(day, month, year)
  set_all(0,0,0)

  
def buttonB_wasPressed():
  global state
  if state == 'B':
    set_state_a()
  else:
    set_state_b()

def buttonC_wasPressed():
  global study_array, sum_label, average_label, max_label, min_label
  set_state_c()
  sum_seven_days = 0
  if len(study_array) <= 6:
    for val in study_array:
      sum_seven_days += val
  else:
    for i in range(7):
      sum_seven_days += study_array[len(sum_seven_days) - i - 1]
  avr = sum_seven_days // 7
  sum_arr = sum(study_array) / len(study_array)
  max_arr = max(study_array)
  min_arr = min(study_array)
  cal_hr_str("Average study time last 7 days: ", average_label, avr)
  cal_hr_str("Average study time: ", sum_label, sum_arr)
  cal_hr_str("Max study time: ", max_label, max_arr)
  cal_hr_str("Min study time: ", min_label, min_arr)

btnA.wasPressed(buttonA_wasPressed)
btnB.wasPressed(buttonB_wasPressed)
btnC.wasPressed(buttonC_wasPressed)

setScreenColor(0x222222)

ultrasonic = unit.get(unit.ULTRASONIC, unit.PORTA)
adc_pin = machine.Pin(36)  
adc = machine.ADC(adc_pin)
adc.atten(machine.ADC.ATTN_11DB)
adc.width(machine.ADC.WIDTH_12BIT) 

np = neopixel.NeoPixel(machine.Pin(26), 3)  

date_label = M5TextBox(35, 60, "DD MMM YYYY", lcd.FONT_DejaVu40, 0xFFFFFF, rotate=0)
welcoming_label1 = M5TextBox(35, 150, "What a great day!", lcd.FONT_DejaVu18, 0xFFFFFF, rotate=0)
welcoming_label2 = M5TextBox(35, 180, "It's a great time to study!", lcd.FONT_DejaVu18, 0xFFFFFF, rotate=0)
studying_label = M5TextBox(50, 60, "Happy stuyding!", lcd.FONT_DejaVu24, 0xFFFFFF, rotate=0)
time_label = M5TextBox(60, 120, "00:00:00", lcd.FONT_DejaVu40, 0xFFFFFF, rotate=0)

average_label = M5TextBox(0, 60, "", lcd.FONT_Default, 0xFFFFFF, rotate=0)
sum_label = M5TextBox(0, 75, "", lcd.FONT_Default, 0xFFFFFF, rotate=0)
max_label = M5TextBox(0, 120, "", lcd.FONT_DejaVu18, 0xFFFFFF, rotate=0)
min_label = M5TextBox(0, 140, "", lcd.FONT_DejaVu18, 0xFFFFFF, rotate=0)

TARGET_AMBIENT = 1500 
MIN_DIST, MAX_DIST = 40, 74
NORM = MAX_DIST  ** 2 * TARGET_AMBIENT / 255
day, month, year = 20, 5, 2025
study_seconds = 0.0
month_converter = ['NaN', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']
state = 'A'
study_array = []
set_state_a()
cal_date_str(day, month, year)

while True:
  if state == 'B':
    start = time.ticks_ms()
    raw_value = adc.read()
    brightness = 4095 - raw_value
    dist = ultrasonic.distance / 10
    if state != 'B' or brightness >= TARGET_AMBIENT:
      set_all(0,0,0)
    else:
      brigtness_need = TARGET_AMBIENT - brightness
      value = round(dist ** 2 * brigtness_need / NORM)
      set_all(value, value, value)
    end = time.ticks_ms()
    study_seconds += end - start
    # There is a problem when you swtich the states too fast, the display ended up bugging.
    if state == 'B':
      cal_hr_str("",time_label, study_seconds / 1000)
\$\endgroup\$

3 Answers 3

Reset to default
3
\$\begingroup\$

Layout

The code uses 2-space indent, but it is easier to read and understand using the more common 4-space indentation. The black program can be used to automatically indent the code for you.

Globals

It would be better to avoid the use of global. For example, in the set_state_a function, you could pass state into the function, then return it. Since you make extensive use of globals, this might be a case where you could consider using classes.

Documentation

The PEP 8 style guide recommends adding docstrings for functions and at the top of the code. For example, you could summarize the purpose of the code as you do in the question:

"""
An automatic light-adjusting lamp using M5GO.

The M5GO bot has 3 buttons:
etc.
"""

Naming

The variable named min in the cal_hr_str function is the same name as a Python built-in function. This can be confusing. To eliminate the confusion, rename the variable as something more specific, like "minute". The first clue is that "min" has special coloring (syntax highlighting) in the question.

DRY

Code like this is repeated several times:

day_text = str(day) if day >= 10 else "0" + str(day)

You could create a function to convert a number to a string.

Logic

The while loop only seems to do anything in the B state. Also, since you set the state to A right before the loop, it looks like the loop will never do anything:

state = "A"
//...
set_state_a()
//...

while True:
    if state == "B":

This is confusing. It would be worth adding comments to explain the behavior.

\$\endgroup\$
3
\$\begingroup\$

I won't repeat what @toolic has already said.

Efficient String Operations

The way to convert an int for display so that it is left-padded with zeroes is:

x = 6
...  # other code
s = str(x).zfill(2)

You can also use f-strings:

x = 6
...  # other code
s = f'{x:2}'

F-strings become useful when you want to avoid unnecessary string concatenation operations. For example:

def cal_date_str(day, month, year):
    ...  # code omitted
    #date_label.setText(day_text+ ' ' + month_text + ' '+ year_text)
    date_label.setText(f'{day:2} {month_text} {year}')

Use the datetime Module

A lot of your code could be simplified if you use the available tools provided in the standard library. Here is how you could be performing "date math" and formatting dates:

import datetime
from dateutil import relativedelta

def format_date(dt: datetime.date) -> str:
    return dt.strftime('%b %d, %Y')

day, month, year = 20, 5, 2025

dt1 = datetime.date(year, month, day)
print(format_date(dt1))  # 'May 20, 2025'

# Calculate next day's date:
dt2 = dt1 + datetime.timedelta(days=1)
print(format_date(dt2))  # 'May 21, 2025'

# Calculate first day of following month:
dt3 = dt1 + relativedelta.relativedelta(months=1, day=1)
print(format_date(dt3))  # 'Jun 01, 2025'

Consider Using OOP

I think your code be more readable and more easily maintained if you encapsulated your global data and the functions that manipulate it into one or more classes where the global data are attributes and the functions become methods.

\$\endgroup\$
2
\$\begingroup\$

I am assuming this is MicroPython, which I am not much familiar with, but the remarks below are valid for Python in general.

Import

import * is not good practice, because it pollutes your namespace with stuff you don't need, and can cause namespace confusion. Meaning: this could overwrite a function that is already defined elsewhere.

Unfortunately, you may see a lot of tutorials/sample code using that pattern, but it still not recommended.

As @toolic mentions, you already have one such issue (shadowing) with the min function. A good IDE should highlight this kind of problematic code.

The choice is yours: either do: from m5stack import this, that or: import m5stack. And then, you prefix all the functions you use from that package. Like you are doing with machine.

Naming

The function names are not very telling. I would think about more descriptive (longer) names. Then, a bit of documentation (docstrings) would be welcome. Not just for outsiders, but also for yourself. When you revisit the code in 6 months, you might struggle understanding the intent behind some of code you wrote.

For example, I have no idea what set_all does, because the name is too generic and does not tell anything about its purpose. Even looking at the code which is very short, I still need to scroll over the rest of the code to finally get a vague idea.

DRY

There is a lot of repeat code, that makes the code unnecessary long. set_state_a, set_state_b, set_state_c should be merged into one single function. Depending on the value of state, you will want to hide or show a label, but there is little difference in behavior otherwise. All you need is a simple if or match block.

Date

It seems that the datetime module is not available in MicroPython, but the code can still be simplified using the built-in modules.

Example: Add one day to the current date:

import time
# Get the current time
today = time.localtime()
# Add one day (in seconds)
one_day = 24 * 60 * 60
tomorrow = time.mktime(today) + one_day

Source: https://micropython-tips.weebly.com/time-and-date.html

Here you have an overview of the standard libraries for MicroPython. I suggest you always have a look here first, to see what is available before reinventing the wheel.

As to the formatting, I suppose the F-strings are available in MicroPython and should be sufficient to output datetime values in the format you want.

Constants

Consider defining a few constants for colors eg. 0x222222. I know that 0xFFFFFF should be white and 0x00000 is black, but I don't know about 0x222222. I need to look this up to see what it looks like.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.