Coding Guidelines

From ZynthianWiki
Revision as of 22:40, 12 November 2026 by Riban (talk | contribs) (→‎Formatting)
Jump to navigation Jump to search

1 Zynthian UI (tomita branch) Coding Guidelines

1.1 About these guidelines

These guidelines record how code in the tomita branch of zynthian-ui is written today, and where new code should do better. They come from measuring the branch at commit ec38132 (11 November 2026): 185 Python files (about 92,000 lines) plus C and C++ libraries under zynlibs/. They are advice drawn from a code survey, not an official Zynthian project policy.

Note: This guide is drafted by claude.ai using zynthian-ui repository as its source and includes statistics about the current level of conformity. Further review of this guide and existing code is in progress but you may use this guide as the current rule-set.

The branch has no style configuration: no .editorconfig, linter or formatter settings, and no contributing guide. The conventions live in the code itself, so each rule below says how common the practice is and you can judge how firm it is.

  • Must: near-universal practice, or it prevents real bugs.
  • Should: majority practice, or clearly better practice for new code.
  • Avoid: present in the code but discouraged. Do not copy it.

When you edit an existing file, match that file's style and change only the lines your work needs. Do not reformat or rename unrelated code: large cosmetic diffs make review harder and make merges between branches painful.

1.2 Repository layout

Put new code where its kind already lives. The split between drawing code in zyngui/ and logic elsewhere is worth protecting: no module under zyngine/, zynautoconnect/, zynconf/ or zynlibs/ imports Tk today, and new code there must keep it that way.

Path Holds
zyngui/ Tkinter screens (zynthian_gui_*.py), engine widgets (zynthian_widget_*.py), zynthian_gui_config.py and the main controller zynthian_gui.py
zyngine/ State manager, chain manager, processors, signal manager and one wrapper per synth engine (zynthian_engine_*.py)
zyngine/ctrldev/ MIDI controller drivers (zynthian_ctrldev_*.py)
zynautoconnect/ Automatic connection of JACK and MIDI ports
zynconf/ Configuration helpers
zynlibs/ Native libraries with Python wrappers: zynseq, zynsmf, zynmixer, zynaudioplayer, zynclippy
zyncoder/ Symlink to a sibling checkout of the hardware library, imported as zyncoder.zyncore
help/, icons/, img/, fonts/ Assets
Top level Entry points (zynthian_main.py, zynthian_headless.py), the state schema (zynthian_state_schema.py) and shell scripts

Where a new file goes:

  • A screen: zyngui/zynthian_gui_<name>.py, holding class zynthian_gui_<name>.
  • An engine widget: zyngui/zynthian_widget_<name>.py.
  • A synth engine wrapper: zyngine/zynthian_engine_<name>.py.
  • A controller driver: zyngine/ctrldev/zynthian_ctrldev_<vendor>_<model>.py.

1.3 Python file layout

Start every new module with the project banner. Use your own name and the year the file was created, and extend the end year of an existing range when you change a file, e.g. 2020-2026.

# ******************************************************************************
# ZYNTHIAN PROJECT: <area, e.g. Zynthian GUI>
#
# <One-line description of the module>
#
# Copyright (C) <year> <your name> <your email>
#
# ******************************************************************************
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License as
# published by the Free Software Foundation; either version 2 of
# the License, or any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# For a full copy of the GNU General Public License see the LICENSE.txt file.
#
# ******************************************************************************

After the banner, order a module like this:

  1. Imports (see Imports below).
  2. Module-level constants in UPPER_CASE.
  3. Module-level helper functions.
  4. The main class, named like the module.

The filename should normally be the same as the primary class name.

Avoid doing real work at import time; module-level constants and cheap singletons such as the signal manager instance are the norm.

1.4 Naming

Project code should use snake case, including class names. Python follows this table:

Item Convention Notes
Module files zynthian_<area>_<name>.py, snake_case Areas in use: gui, ctrldev, engine, widget.
Classes snake_case, e.g. zynthian_gui_base Some existing helper classes break this rule and should ideally be fixed.
Functions and methods snake_case Lowercase
Private members Leading underscore Lowercase
Constants SNAKE_CASE Uppercase
Event handlers <name>_cb on_<event> and cb_<name> also exist and should ideally be fixed.
CUIA actions cuia_<action> Methods defined in zyngui.
Signals S_<AREA> and SS_<AREA>_<EVENT> Defined in zynthian_signal_manager.py.
Environment variables ZYNTHIAN_<NAME>

Choose descriptive names and avoid shadowing built-ins, e.g. prefer img_id to avoid shadowing id() built-in function.

In C and C++ under zynlibs/, classes are CamelCase (Pattern), methods are camelCase and member variables have a m_ prefix, often followed by a type letter (m_nBeats, m_fSwingAmount). Functions exported to Python keep the camel-case names, because the ctypes wrapper binds to them by name.

1.5 Formatting

No formatter or linter is configured, so format by hand and only on the lines you change. Do not run an auto-formatter over a whole file: it would rewrite thousands of lines and bury your change.

  • Indentation. Use 4 spaces, never tabs.
  • Line length. Try to keep lines no longer than 80 characters but do not wrap if just longer.
  • Quotes. Use double quotes for strings. There are many single quotes used but these should ideally be fixed.
  • String building. Use f-strings. .format() and % are older style that have been used and should ideally be fixed.
  • Blank lines. Two between top-level definitions and one between methods, as PEP 8 says. There are several existing files that need fixing.
  • Line endings and whitespace. Use LF endings and leave no trailing whitespace on lines.
  • Comparisons. You must compare with is None / is not None, and test booleans directly or use is True if variable may be other values.
  • Commented-out code. Delete it; git keeps the history.
  • Markers. Write TODO: or FIXME: with what is missing and why.
  • Mutable defaults and globals. You must not use [] or {} as a default argument.

A .editorconfig such as this one in your own checkout keeps most editors consistent.

root = true

[*.{py,c,cpp,h}]
indent_style = space
indent_size = 4
end_of_line = lf
insert_final_newline = true
trim-on-save = true

1.6 Docstrings and comments

Use triple double quotes (1,664 openers against 10 with single quotes). Only about one definition in five has a docstring today (926 of 4,499), so documentation is where new code can most easily raise the standard.

  • Give every new module, class, and any method that other files call or subclasses override a docstring.
  • Start with a one-line summary directly after the opening quotes (626 docstrings do; 233 add a space). Add a blank line before any detail.
  • Describe arguments in an Args: section and results in Returns: (94 and 73 uses). The older free-form name : description lists, as in add_processor(), are less common; do not extend them to new code.
  • State units, valid ranges, what is returned on failure, and which thread may call the method. run_script() in zynthian_gui_dpm.py does the last well: it notes that it must be called from the Tk main thread.
def norm_to_db(norm, range_db=60):
    """Convert a normalised meter position to decibels.

    Args:
        norm: Position from 0.0 (bottom of the meter) to 1.0 (0 dBFS)
        range_db: Span of the meter in dB

    Returns:
        Level in dB, from -range_db to 0
    """

That example is for illustration only; it is not a function in the branch.

Comments explain why, not what. Say so when code must stay in step with something elsewhere: db_to_norm() carries the note "Match logCurve() in zynmixer lib", which saves the next reader a long search. Put trailing comments after two spaces (daemon = True # thread dies with the program). Update or delete a comment when you change the code beside it.

1.7 Imports

Group imports as standard library, third-party packages, then project modules. 108 files start the project group with the comment # Zynthian specific modules; copy that habit. Order within a group is not enforced.

import copy
import logging
import tkinter
from threading import Timer

from PIL import Image, ImageDraw

# Zynthian specific modules
from zyncoder.zyncore import lib_zyncore
from zyngui import zynthian_gui_config
from zyngui.zynthian_gui_base import zynthian_gui_base
from zyngine.zynthian_signal_manager import zynsigman
  • Absolute imports. You must import from the package root, as the code does in 1,291 import lines. Only 5 relative imports exist.
  • No wildcards. Avoid from X import * (46 uses today, for example from zynlibs.zynaudioplayer import * in the mixer screen). Wildcards hide where a name comes from and invite collisions.
  • Function-level imports. There are 28. Use one only to break a circular import or defer a costly module, and say why in a comment.
  • Tidy up. Remove imports you no longer use and do not leave commented-out ones such as #import traceback. The repo has no linter, so run pyflakes or flake8 locally on files you change. Neither is in requirements.txt.

1.8 Logging and error handling

1.8.1 Logging

Call the logging module directly. The branch makes 1,401 such calls against 32 on a named logger; logging.getLogger() appears mainly to quieten a library, as with logging.getLogger('PIL').setLevel(logging.WARNING). Use logging, never print (26 calls remain).

Level Uses Use it for
debug 544 Developer tracing that is off in normal running
info 239 A normal event worth seeing in the log, such as a start or a load
warning 223 Something unexpected that the code handled
error 368 An operation failed
exception 27 An except block that needs the traceback
  • Put the identifying value in the message and quote it, as logging.error(f"Chain '{chain_id}' doesn't exist!") does.
  • Inside an except block, prefer logging.exception("What failed"). It records the traceback; logging.error(e) keeps only the message. Do not pass it traceback.format_exc() as zynthian_signal_manager.py does, because logging.exception already appends the traceback.
  • Avoid building log text in code that runs per MIDI event, per meter refresh or per audio period. An f-string is formatted even when the level is off. Use lazy arguments there: logging.debug("CC %d on channel %d", ccnum, chan).

1.8.2 Error handling

  • No bare except:. The branch has 623 of them, against 455 except Exception as e and 58 that name a specific exception. A bare except: also catches KeyboardInterrupt and SystemExit and hides typing mistakes, so you must not add one.
  • Catch the narrowest exception you expect, such as KeyError, ValueError, OSError or tkinter.TclError. Use except Exception only at the edge of a thread, callback or event handler, where the program has to carry on, and always log it. run_script() in zynthian_gui_dpm.py is a good model: it catches TclError and logs a warning with context.
  • Failure returns. Functions that can fail log the cause and return None; add_processor() documents "processor object or None on failure". Say so in the docstring and test the result with is None at the call site.
  • Validate at the edges. Check values that arrive from users, files, MIDI, OSC and the network. Trust calls between your own modules.

1.9 GUI views (Tkinter)

Most screens derive from zynthian_gui_base, directly or through the zynthian_gui_selector classes. Override the hooks you need, call super() where the base does work, and keep self.shown accurate. The base class documents its hooks in comments above each method; do the same for any hook you add.

Hook Contract
build_view() Draw the screen ready to display and return True. The base returns True.
show() / hide() Show or hide the view. Guard with self.shown so repeated calls are harmless.
zynpot_cb(i, dval) Rotary controller i moved by dval. Return True when handled.
switch(switch, typ) Legacy switch press. typ is "S", "B" or "L" (short, bold, long). Return True when fully handled, or False to let the parent act.
switch_select(typ) Select press, with the same typ values.
back_action() Return True if the screen handled Back, otherwise False.

1.9.1 Signals

Subscribe through zynsigman (the signal manager). send() calls handlers directly in the sender's thread, and send_queued() or a handler registered with register_queued() runs on the SIGNAL_QUEUE thread, never on the Tk thread.

  • Register in build_view() and unregister in hide(), each guarded by self.shown, as the mixer screen does. The branch has 102 register calls and 97 unregister calls, so some are never undone; a leaked handler keeps running for a hidden screen.
  • Handlers receive keyword arguments, so declare them explicitly or accept **kwargs.

1.9.2 Threads and Tk

Tk is single-threaded. The DPM code notes that tkinter forwards calls from other threads to the main thread and waits for each one, which is slow and can deadlock if the main thread is waiting on you. From a worker or signal thread, schedule widget changes with self.zyngui.after_idle(func, *args), as zynthian_gui_control.py does, instead of touching widgets directly.

1.9.3 Drawing

  • Create canvas items once and update them with coords() and itemconfig() (99 and 278 uses) rather than deleting and recreating them (41 delete() calls).
  • Group items with tags (the mixer tags meter items "dpm" and changes them in one call) and skip updates when the value has not changed.
  • Batch Tcl calls where you can. run_script() in zynthian_gui_dpm.py exists to cut the number of tk.eval() calls.
  • Use after_idle() rather than update(), which re-enters the event loop.
  • Do not name methods after Tk's own. Nine classes define config() and four define update(), which shadow the Tk methods of the same name on any class derived from a Tk widget.

1.9.4 Theming and layout

Take colours, fonts and sizes from zynthian_gui_config (color_bg, color_panel_bg, color_tx, font_family, font_size, display_width and so on) so every layout and colour variant works. Avoid literals such as the outline="yellow" highlight in the mixer launcher; add a named colour to the config instead. Lay out frames with grid() (105 uses; pack() has 30 and place() 12) and draw custom controls on a canvas.

1.10 Threads, timers and concurrency

The code base uses plain threads (38 Thread constructions, 35 of them daemon), queues (7), locks (15), threading.Event (17) and multiprocessing (13). It does not use asyncio. Follow the existing pattern in zynthian_gui.py: name the thread, mark it daemon, start it from a start_* method.

from queue import Queue, Empty
from threading import Thread, Event


def start_worker(self):
    self.worker_exit = Event()
    self.worker_queue = Queue()
    self.worker = Thread(target=self.worker_task, args=())
    self.worker.name = "my_worker"
    self.worker.daemon = True  # thread dies with the program
    self.worker.start()


def worker_task(self):
    while not self.worker_exit.is_set():
        try:
            item = self.worker_queue.get(True, 1)
        except Empty:
            continue
        try:
            self.process(item)
        except Exception:
            logging.exception("Worker failed")


def stop_worker(self):
    self.worker_exit.set()
    self.worker.join(2)

The snippet is an illustration of the pattern, not code from the branch. The signal manager's queue_thread_task() has the same shape: a blocking get() with a one-second timeout so the loop notices its exit flag.

  • Name every thread. Names show up in logs and debuggers.
  • Make loops stoppable. Wait on a queue or Event.wait(timeout) rather than sleeping blindly, and provide an explicit end() or stop() that signals the thread and joins it. Daemon threads die without cleanup, so release ports, files and processes explicitly.
  • Hand work over with a queue. zynthian_gui.py does this for switch events: zynswitch_defered() puts them on cuia_queue and a worker handles them. Prefer this to sharing mutable data between threads.
  • Lock narrowly. Guard state that several threads touch, hold the lock for as short a time as you can, and never call into Tk or zynsigman.send() while holding it.
  • Timers. A threading.Timer callback runs on a new thread, so it must follow the Tk rule above. For delays in GUI code prefer self.after(ms, func), which runs on the Tk thread, and keep the returned id so you can call after_cancel() in hide(). The branch has 40 after() calls but only 11 after_cancel() calls. Cancel a Timer you replace.
  • Do not block event handlers. Run slow work (a subprocess, file or network access) on a worker thread and report back with after_idle().
  • Separate processes. Controller drivers that need one use multiprocessing (see init_midiproc() in zynthian_ctrldev_base.py). Reset signal handlers in the child, as that module does.

1.11 Engines, controller drivers and widgets

1.11.1 Synth engines

Subclass zynthian_engine, call super().__init__(state_manager), then set self.name, self.nickname and self.jackname. Override only the hooks your engine needs. The common ones are get_bank_list(), set_bank(), get_preset_list(), set_preset(), get_controllers_dict(), send_controller_value(), start() and stop().

  • Engines are registered in the dictionary in zynthian_chain_manager.py, keyed by a two-letter code ("SF" for sfizz). A new engine needs an import and an entry there.
  • set_preset() returns a bool saying whether the preset loaded; the engines follow this consistently.
  • Use the helpers in the base class, such as get_filelist(), to list banks and presets instead of writing your own directory walk.
  • Keep Tk out of engines. Engine-specific screens belong in a widget.
  • zynthian_engine_sysex.py is one of the five tab-indented files. Do not copy it as a template without converting it to spaces.

1.11.2 Controller drivers

Subclass zynthian_ctrldev_base, or one of its extended variants in the same folder, and name the file and class zynthian_ctrldev_<vendor>_<model>. The default driver name is the class name minus its 17-character zynthian_ctrldev_ prefix. zynthian_ctrldev_duopiano.py (54 lines) is a minimal, readable model.

Class attribute Purpose
dev_ids Strings that identify the device when it is detected
driver_name, driver_description Optional display text
unroute_from_chains True to take the device off the chains, False to keep it routed, or a bit mask of MIDI channels
autoload_flag False to stop the driver loading itself on detection
dev_zynpad, dev_zynmixer, dev_pated Capability flags
  • Do set-up in init() and clean-up in end(), and call super().end() from your override.
  • Handle input in midi_event() and redraw device lights in refresh(). Save and restore driver state with get_state() and set_state().
  • Send output through lib_zyncore (for example dev_send_note_on()), and use state_manager.add_slow_update_callback() for periodic work rather than a thread of your own.
  • Drivers are loaded and unloaded by zynthian_ctrldev_manager; never create them directly.

1.11.3 Widgets

Subclass zynthian_widget_base (a tkinter.Frame) in zyngui/zynthian_widget_<name>.py. zynthian_gui_control.py loads a widget by file name from the controller screen type, so the file name has to match.

  • Draw in refresh_gui(). The base update() calls get_monitors() and then refresh_gui() only while both the widget and its control screen are shown, so do not override update().
  • refresh_gui() runs often. Create canvas items once and change them only when a monitored value has changed.
  • Resize in on_size(), but catch a specific exception there. The base class's bare except: around the canvas resize is not a pattern to copy.

1.12 State save and restore

Snapshots are JSON files. zynthian_state_schema.py documents them: it holds a commented example of the ZynthianState dictionary and its schema_version (currently 6). The state manager, chain manager, chain, processor, controller, controller drivers and the zynseq wrapper each save with get_state() and restore with set_state().

When you add something that must persist:

  1. Return plain data from get_state(): dictionaries, lists, strings, numbers, booleans and None. It must survive a JSON round trip, so tuples come back as lists (the schema's zctrls example shows tuples).
  2. Read optional keys defensively. The existing set_state() code tests for a key before it uses it, so older snapshots still load. A missing key means use the default.
  3. Document the key in zynthian_state_schema.py with an example value, its units and its default, in the style of the entries already there.
  4. Bump the schema when a change is not backward compatible. Raise SNAPSHOT_SCHEMA_VERSION in zynthian_legacy_snapshot.py and add the next version_<n>() method there. The converter steps through versions one at a time and returns None for a snapshot newer than the code. version_5() shows the pattern.
  5. Keep runtime values out: widget references, timers, file handles and anything else that cannot be rebuilt from the data.
  6. Let a damaged snapshot load as far as it can. Catch errors per item, log them with logging.exception() and carry on, so one bad entry does not stop the rest of the restore.

1.13 Native code (C and C++)

The libraries under zynlibs/ (zynseq, zynsmf, zynmixer, zynaudioplayer, zynclippy) are built with CMake and used from Python through ctypes. Run zynlibs/build.sh [Release|Debug] to build them all (the default is Release), or the build.sh inside one library folder.

1.13.1 Style

  • Indent with 4 spaces and put the opening brace on the same line, as pattern.cpp does.
  • Start each file with the /* ... */ banner used in zynseq.h and mixer.c: project name, description, copyright and the GPL text.
  • Document public functions with Doxygen blocks. The libraries have 689 /** */ blocks and 991 @param lines, in the form @brief, @param, @retval.
  • Put #pragma once in every header. 10 of 16 do; one uses an #ifndef guard and five have none.
  • Use braces on every if and loop body in new code. The single-statement form exists, but it invites mistakes when someone adds a line.
  • Give #define constants an UPPER_CASE name and a trailing comment with the unit and the reason, as mixer.c does for DPM_RANGE_DB. In C++ prefer constexpr.
  • For debug output use the library's DPRINTF macro. In zynseq.cpp it prints to stderr only when g_bDebug is set. Bare printf calls (144 today) are noise in a running system.

1.13.2 Build

Most libraries compile with -Werror, so new code must build without warnings. Add -Wall to your local build; only zynaudioplayer enables it. Do not hard-code the build type: zynseq/CMakeLists.txt sets CMAKE_BUILD_TYPE to Debug, which overrides the Release default that build.sh passes.

1.13.3 Real-time code

The JACK process callbacks (onJackProcess() in zynseq, zynsmf and zynmixer) run on a real-time thread. This is standard JACK practice rather than a pattern measured in the branch. Inside them:

  • Do not allocate memory, take a lock that can block, write to files or stdio, or call into Python.
  • Keep every loop bounded.
  • Share data with other threads through atomics or a lock-free hand-off.

1.13.4 The ctypes boundary

  • Export functions inside extern "C", guarded by #ifdef __cplusplus, using plain C types: fixed-width integers, float and const char*. Do not let C++ types, references or exceptions cross.
  • Python binds to exported functions by name, so renaming one breaks the wrapper at run time, not at build time. Change the header, the .cpp and the Python wrapper together.
  • Set argtypes on every function you add to a wrapper, and set restype unless it returns int. ctypes assumes int, which is wrong for floats, pointers and 64-bit values. zynseq.py makes 66 such assignments against roughly 210 prototypes in zynseq.h, so many functions rely on the defaults; do not add to that.
  • Return pointers that stay valid after the call (static or member storage), because Python reads them afterwards.

1.14 Security, robustness and file handling

  • Subprocesses. shell=True appears 42 times, including check_output(f"systemctl start {service}", shell=True) in zynthian_state_manager.py. Pass an argument list instead: check_output(["systemctl", "start", service]). It needs no quoting and a stray character in a name cannot change the command. If you really need a shell, for a pipe say, wrap every inserted value in shlex.quote(); nothing in the branch uses it yet. Replace os.system() (5 uses) with subprocess too.
  • eval() and exec(). zynthian_state_manager.py and zynthian_gui.py both evaluate "zynthian_gui_config.{}".format(varname). Write getattr(zynthian_gui_config, varname) instead, and never evaluate data from files, MIDI, OSC or the network.
  • Files. Open them with with open(...) (71 uses; 7 assign the handle and rely on closing it later). Pass encoding="utf-8" for text: only 2 of the 92 open() calls do.
  • Paths. Build them with os.path.join() or pathlib, not string concatenation, and take base folders from the environment: ZYNTHIAN_UI_DIR, ZYNTHIAN_MY_DATA_DIR, ZYNTHIAN_CONFIG_DIR, ZYNTHIAN_DATA_DIR and the others the code already reads. The branch has 66 hard-coded /zynthian/... string literals in 41 files, which stops the code running from anywhere else.
  • Environment variables. Read them with os.environ.get("ZYNTHIAN_X", default) (143 uses) rather than os.environ["ZYNTHIAN_X"] (11), which raises KeyError. Values are strings: convert and range-check them.
  • External input. Check the type, range and length of anything that arrives from MIDI, OSC, the network, files or the user before you use it. Reject file names that contain path separators or .. before joining them to a folder.
  • Secrets. Do not log or commit passwords, keys or Wi-Fi credentials, and keep them off command lines, which any user can see in ps.
  • Resources. Release what you acquire (files, JACK ports and clients, child processes) in an explicit end() or stop().

1.15 Performance on Raspberry Pi hardware

Zynthian runs on a Raspberry Pi, where the Python UI shares a small CPU budget with synth engines and audio processing. The UI must never be the reason audio glitches, so treat every periodic or per-event code path as hot.

  • Measure on the device. A desktop hides most problems. Time sections with time.monotonic() (48 uses) rather than time.time() (32), which can jump when the clock is set.
  • Hook into the single refresh loop. zynthian_gui.py calls the current screen's optional plot_zctrls() and refresh_dpm() once per pass, and skips the meters in power-save mode. Add periodic drawing there instead of starting a timer per widget.
  • Coalesce fast sources. A flood of MIDI CC or meter updates should store the latest value and let the refresh pass draw it once, not redraw per message.
  • Draw only what changed. Compare against the last value and update the canvas items that differ. See the Drawing notes under GUI views.
  • Cache generated images. The mixer keeps a bg_images dictionary keyed by size so each background is rendered once. Do the same for anything built with PIL.
  • Never busy-wait. Avoid while True: sleep(0.01) loops. Wait on a queue or an Event with a timeout.
  • Keep per-sample work native. Anything that runs per audio sample or per audio period belongs in a zynlibs library, not in Python.
  • Mind the SD card. Save on a change or a user action, not in a loop, and never write files inside an event handler that has to stay responsive.
  • Log with care. See Logging: formatting a message costs time even when the level is off.

1.16 Testing

Automated coverage is thin. Seven files have test in their name, five import unittest and none use pytest. Nothing tests the state manager, chain manager, engines or screens. The one suite that run_tests.sh runs lives in zynlibs/zynseq/tests and is a model for new work.

  • Use unittest, with classes named Test<Thing> and methods named test_<what>.
  • Make tests independent. zynlibs/zynseq/unit_tests.py orders its tests with names like test_aa01_loadfile, so one failure cascades. Do not copy that.
  • Test timing against frames, not the wall clock. The zynseq tests run the real library on a private JACK server with the dummy backend in freewheel mode, so 40 seconds of audio take a fraction of a second and results are repeatable. Reuse the helpers in jacktest.py (JackTestServer, FreewheelRunner, MidiSource, Recorder). The folder's README explains the setup. Build the library first and avoid running the suite while playing live, because freewheel keeps a CPU core busy.
  • Do not hard-code device paths. unit_tests.py loads /zynthian/zynthian-ui/zynlibs/zynseq/build/libzynseq.so and writes to /tmp/test.zynseq. Locate files relative to the test, and use tempfile for output.
  • Keep logic out of Tk code so it can be tested without a display. Pure functions such as db_to_norm() are easy to test.
  • Run the suite with ./create_venv.sh once, then ./run_tests.sh, before submitting any change to zynseq.
  • Try UI changes on a device or desktop install. source zynthian_debug.sh sets ZYNTHIAN_LOG_LEVEL=10 for debug logging and ZYNTHIAN_UI_POWER_SAVE_MINUTES to 0, then starts the UI. Exercise the screens you touched, including back and long-press paths, and check the log for new errors and warnings.

1.17 Commits, patches and pull requests

The last 60 non-merge commits on tomita set the pattern.

  • Subject line. One sentence-case line, usually starting with a verb: Fixes (10 commits), Fix (9) and Add (8) lead. 49 of 60 end with a full stop. The median length is 38 characters and the 90th percentile 72, so keep subjects under about 72. An area prefix such as Mixer: or zynseq: is fine when it helps.
  • Body. 14 of 60 commits have one. Write a body when the reason is not obvious: a behaviour change, a unit or range change, a trade-off, a measurement.
  • Issues. Refer to the tracker entry when there is one, for example Fixes zynthian/zynthian-issue-tracking#1825. Only 3 of 60 do, so this is worth doing more often.
  • One logical change per commit. Run git status before committing so that new files are not left behind.
  • Branches. Feature branches are named <issue number>/<topic> (1820/midi_binding, 1822/fader_law) and reach tomita through pull requests. The history shows contributors merging tomita into their branch to stay current; do that unless a maintainer asks for a rebase.
  • Keep generated and private files out. .gitignore already covers venv, __pycache__, zynlibs/*/build, editor folders and *.APIKEY and *.TOKEN files.
  • Keep the diff small. Do not mix formatting, renaming or file moves with a functional change. Reviewers cannot see the change for the noise.

A pull request description should say what changed and why, how you tested it (device or desktop, and what you exercised), and whether it changes the snapshot schema, adds an environment variable or alters a native library interface. Add a photo or screenshot for any visible GUI change.

1.18 Pre-submission checklist

  • ☐ My change matches the style of the file it is in, and I reformatted only lines I changed.
  • ☐ New files have the banner, a name like zynthian_<area>_<name>.py and a class named after the module.
  • ☐ Indentation is 4 spaces, line endings are LF, and there are no tabs or trailing whitespace.
  • ☐ There is no bare except:, errors are logged with context, and I compare with is None.
  • ☐ I used logging, not print, and format nothing expensive in hot paths.
  • ☐ New classes and overridable methods have docstrings with Args: and Returns:.
  • ☐ Signal handlers are registered in build_view() and unregistered in hide().
  • ☐ Tk is touched only from the Tk thread (after_idle() from workers), and timers are cancelled in hide().
  • ☐ Threads are named, daemon and stoppable, and nothing busy-waits.
  • ☐ Persistent state goes through get_state() and set_state(), the schema file is updated, and the version is bumped if the change is not backward compatible.
  • ☐ There is no shell=True with interpolated values and no eval(), folders come from environment variables, and text files open with with open(..., encoding="utf-8").
  • ☐ Native code builds without warnings, onJackProcess() does not allocate or block, and new ctypes functions have argtypes and restype.
  • ☐ I tested on a device or desktop install, and run_tests.sh passes if I touched zynseq.
  • ☐ The commit subject is under about 72 characters, covers one logical change and cites the issue, and the pull request says how I tested it.