Split flap clock stepper code

Question

I've built a Split Flap Clock, and the code has grown quite a lot, and although it does work in its current state, I am wondering if it could be cleaned up.

The whole thing runs on an ESP32 and is written in micropython.

Some information about the system:

• A digit has twelve flaps on a spool (twelve to also be able to build a calendar with 12 months).
• The spool has a magnet attached to it which a hall sensor can detect.
• The Stepper 28BYJ-48 has a total of 512 steps (unfortunately not divisible by 12).
• Upon calibration the spool rotates until the range where the magnet is active is found. The middle of that range is the calibration point.
• The calibration point plus or minus a number of steps is the zero point where the first digit (here a 0) is visible.
• After some rotations the stepper would run out of sync with the presumptive position so the advance method performs an "on-the-fly-calibration" when the digit goes past the magnet position.
• The digit should only move if it is calibrated.
• The digits are calibrated one after the other and as soon as a digit is calibrated it should show the correct value right away.

I chose to put all the functions related to stepper movement into the Digit class. So I can use the digit as follows:

c1 = -10 # correction steps after calibration so that the first digit is visible
s1 = Stepper(HALF_STEP, Pin(10, Pin.OUT), Pin(9, Pin.OUT), Pin(3, Pin.OUT), Pin(8, Pin.OUT), d) # initialise stepper class
h1 = Pin(18, Pin.IN) # hall sensor Pin
d1 = Digit(s1, h1, list(range(0,12)), c1, 1, label='Hour 1')
d1.show(1)

digit.py

from uln2003 import Stepper, HALF_STEP, FULL_ROTATION
import time
HALL_ACTIVE = 0

class Digit():
    def __init__(self, stepper, sensor, labels = [], offset = 0, direction = 1, label =''):
        self.stepper = stepper
        self.sensor = sensor
        self.labels = labels
        self.direction = direction
        self.position = -99999
        self.offset = offset
        self.label = label
        self.magnet_range = 0
        self.correction = 0
    
    def hall_active(self):
        return self.sensor() == HALL_ACTIVE

    def advance_to(self, position):
        position = round(position)
        print(f"Advance from {self.position} to {position}")
        if position > self.position:
            self.advance(position - self.position)
        elif position == round(self.position):
            print("Already there")
        else:
            print(f"Advance over zero to {FULL_ROTATION - self.position + position}")
            self.advance(FULL_ROTATION - self.position + position) 

    def show(self, label):
        if label in self.labels:
            i = self.labels.index(label)
            self.advance_to(i * FULL_ROTATION / len(self.labels))

    def advance(self, steps):
        steps_left = round(steps - self.correction) % FULL_ROTATION
        self.correction = 0
        old = self.position
        self.position += steps
        if self.position < 0:
            self.position += FULL_ROTATION
            print(f"Added FULL_ROTATION to get to {self.position}")
        if self.position >= FULL_ROTATION:
            self.position -= FULL_ROTATION
            print(f"Removed FULL_ROTATION to get to {self.position}")
        start = -1
        end = -1
        
        while steps_left > 0:
            i = steps - steps_left
            self.stepper.step(1, self.direction)
            if self.hall_active() and start == -1:
               start = old + i
            if not self.hall_active() and start != -1:
                end = old + i 
                self.correction = round(FULL_ROTATION - (end - (end-start) / 2) - self.offset) % FULL_ROTATION
                start = -1
            steps_left -= 1
        correction_string = f"correction of {self.correction}" if abs(self.correction) > 0 else ''
        print(f"Went: {steps} steps from: {old} to: {self.position} {correction_string}") 


    def calibrate(self, move_to_first = False):
        print(f"Calibrating the {self.label} digit")
        print(f"{self.labels}")

        if self.hall_active():
            i = 0
            while self.hall_active():
                self.stepper.step(1, -self.direction)
                i += 1
            print(f"moved {i} out of the magnet area")

        i = 0
        print(f"Starting calibration")
        while not self.hall_active():
            self.stepper.step(1, self.direction)
            i += 1
            
        print(f"Found the magnet after {i} steps")
        i = 0
        while self.hall_active():
            i += 1
            self.stepper.step(1,self.direction)
        print(f"Reached end of hall sensor at {i}")
        self.magnet_range = i
        # Go to the magnet center
        self.stepper.step(int(i / 2), -self.direction)
        print("Set the absolute zero relative to the offset")
        self.position = FULL_ROTATION - self.offset
            
        if move_to_first:
            print("Go to offset")
            self.advance(self.offset)
        self.position = self.position % FULL_ROTATION
        print(f"calibration ended at position {self.position}")

if __name__ == '__main__':
    from machine import Pin  
    s1 = Stepper(HALF_STEP, Pin(10, Pin.OUT), Pin(9, Pin.OUT), Pin(3, Pin.OUT), Pin(8, Pin.OUT), 0.001)
    d1 = Digit(s1, Pin(18, Pin.IN), list(range(0,12)), -10, 1, label='Weekdays')
    d1.calibrate()
    d1.show(0)

main.py

Using this code, I can display the current time like so:

from machine import Pin
from machine import ADC
import machine
import time
from digit import Digit
import neopixel

from uln2003 import Stepper, HALF_STEP, FULL_STEP, FULL_ROTATION, Driver, Command
from machine import Pin

d = 0.001
BLANK=11

(year,month,mday,h,m,s,weekday,yearday) = time.localtime()

c1 = -12
s1 = Stepper(HALF_STEP, Pin(10, Pin.OUT), Pin(9, Pin.OUT), Pin(3, Pin.OUT), Pin(8, Pin.OUT), d)
h1 = Pin(18, Pin.IN)
d1 = Digit(s1, h1, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], c1, 1, label='Weekdays')

c2 = -8
s2 = Stepper(HALF_STEP, Pin(13, Pin.OUT), Pin(14, Pin.OUT), Pin(21, Pin.OUT), Pin(11, Pin.OUT), d)
h2 = Pin(12,Pin.IN)
d2 = Digit(s2, h2, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], c2, 1, label='Days10')

c3 = -8
s3 = Stepper(HALF_STEP, Pin(40, Pin.OUT), Pin(39, Pin.OUT), Pin(38, Pin.OUT), Pin(6, Pin.OUT), d)
h3 = Pin(47, Pin.IN)
d3 = Digit(s3, h3, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], c3, 1, label='Days')

c4 = -46
s4 = Stepper(HALF_STEP, Pin(1, Pin.OUT), Pin(2, Pin.OUT), Pin(42, Pin.OUT), Pin(41, Pin.OUT), d)
h4 = Pin(48, Pin.IN)
d4 = Digit(s4, h4, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], c4, 1, label='Months')

def showTime():
    (year,month,mday,h,m,s,weekday,yearday) = time.localtime()
    hour = int(h/10)
    if hour == 0: 
        d1.show(BLANK)
    else:
        d1.show(hour)
    d2.show(h%10)
    d3.show(int(m/10))
    d4.show(m%10)

d1.calibrate(); showTime()
d2.calibrate(); showTime()
d3.calibrate(); showTime()
d4.calibrate(); showTime()

while True:
    showTime()
    time.sleep(10)

Answer 1

Quick thoughts

Logging

You have explicitly said you are using Micropython, and that must be the reason why you did not use the logging module. There are lightweight alternatives though (Google search), and I would seriously consider them. For applications that run unattended it is even more important to have log files for control and debugging purposes.

Loops

Rather than increment a variable in a loop, you can simply use the enumerate function.

Imports

There are repeated or superfluous imports eg. from machine import Pin. As already said, I would indeed recommend using ruff which can act as a linter to reformat your code, but also spot the unused imports, which would keep the code tidier. I use pre-commit hooks, so I must fix those defects and I can't commit garbage.

Class

hall_active could be made a class property

Unpacking

In showTime:

localtime probably has a different signature in Micropython, that being said:

(year,month,mday,h,m,s,weekday,yearday) = time.localtime()

Parentheses are not necessary to collect the tuple. In addition, you can assign unneeded values to the anonymous _ variable, thus this statement should be equivalent and you only fetch the two variables you need:

_, _, _, h, m, _, _, _ = time.localtime()

Possibly less readable, I admit but useful to know. On the other hand, if you are running a constrained environment it makes sense to avoid polluting the namespace with unused resources. One more reason to review your imports.

Note that in "standard" Python time.localtime() returns a named tuple of nine components. So, to fetch the hour component, you would just do:

local_time_tuple = time.localtime()
print(f"Hour: {local_time_tuple.tm_hour}")

Answer 2

Follow Useful PEP8 Style Guide Suggestions

For example:

1. Add docstrings for each module and "public" functions. Either in the function docstring, or a function comment or type hints let callers know what argument types are expected and what type of value is returned. Also, add comments where they could be helpful in understanding what logic is being performed.
2. Code import statements in the following order: standard library imports, related third party imports, local application imports. You should use a blank line between these groups of imports.
3. "Private" functions, methods and class attributes should be named with a leading underscore to let clients of the code know that they are know that they should not be accessing them. Use read-only properties for class attributes that a client can access but should never modify.
4. Try not to have lines longer than 79 characters.

Use the Logging facility for Python

You have many statements like print("Already there"), which appear to me to be for debugging purposes. Use the logging API for better control over what messages are displayed under what circumstances.

Remove Unnecessary Checks That Serve Only to Confuse

For example, you have the following (I have added comments to label the two comparisons):

    def advance(self, steps):
        ...
        if self.position < 0:  # Comparison no. 1
            self.position += FULL_ROTATION
            print(f"Added FULL_ROTATION to get to {self.position}")
        if self.position >= FULL_ROTATION:  # Comparison no. 2
            self.position -= FULL_ROTATION
            print(f"Removed FULL_ROTATION to get to {self.position}")
        ...

If Comparison no. 1 is True, then self.position += FULL_ROTATION is computed. But because self.position was a negative value before the addition, we know that the new value for self.position is such that self.position < FULL_ROTATION. So if Comparison no. 1 is True, then Comparison no. 2 can never be True:

    def advance(self, steps):
        ...
        if self.position < 0:
            self.position += FULL_ROTATION
            print(f"Added FULL_ROTATION to get to {self.position}")
        elif self.position >= FULL_ROTATION:
            self.position -= FULL_ROTATION
            print(f"Removed FULL_ROTATION to get to {self.position}")
        ...

Answer 3

A few notes:

• There are no documentation comments.
• You have one case of inconsistent indentation. Not a big deal.
• It's unusual to have your from machine import Pin near the end of your code, rather than at the top with the other imports.
• If you're using Python 3, int(i / 2) can be i // 2. The fact that you've used f-strings suggests you definitely are using Python 3.
• In some cases where you increment i in a while loop, the increment happens first, and in some cases afterwards. It doesn't appear to have any impact on the functionality of your program, but consistency is probably not a bad idea.
• self.position = self.position % FULL_ROTATION is more tersely but equally expressively self.position %= FULL_ROTATION
• print(f"{self.labels}") could just be print(self.labels)

Answer 4

The first thing I see missing from your .py files is ...

Documentation

The PEP 8 style guide recommends adding docstrings for classes and functions. You can also add a docstring at the top of each file to summarize its purpose. Some of the text description in your question would be suitable.

For functions, it is helpful to describe input and return types. Also consider using type hints for the functions to make the code more self-documenting.

Tools

You could run code development tools to automatically find some style issues with your code.

ruff identifies unused import lines, such as this in digit.py:

import time

ruff doesn't complain about the import under the __main__ guard, but it is common to place all import lines at the top of the code

Readability

Large integers are easier to read using underscores. Change:

99999

to:

99_999

In the show function, these lines are a bit awkward:

i = self.labels.index(label)
self.advance_to(i * FULL_ROTATION / len(self.labels))

The variable name i is vague, and it is typically reserved for iterators in a loop. You could combine the 2 lines as:

self.advance_to(self.labels.index(label) * FULL_ROTATION / len(self.labels))

Or, you could use a meaningful name and partition it this way:

new_position = self.labels.index(label) * FULL_ROTATION / len(self.labels)
self.advance_to(new_position)