Difference between revisions of "Coding Guidelines"

From ZynthianWiki
Jump to navigation Jump to search
(Created page with "= Zynthian UI (tomita branch) Coding Guidelines = == About these guidelines == These guidelines record how code in the <code>tomita</code> branch of zynthian-ui is written t...")
 
 
Line 123: Line 123:
 
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.
 
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. The Python tree has 70,732 space-indented lines against 991 tab-indented lines, all in five files (<code>zynthian''engine''midi''control.py</code>, <code>zynthian''engine''setbfree.py</code>, <code>zynthian''engine''sysex.py</code>, <code>zynthian''ctrldev''mackiecontrol.py</code>, <code>zynthian''gui_arranger.py</code>). When you edit one of those, match the surrounding block. C and C++ use spaces too (12,762 lines against 15).
+
* '''Indentation.''' Use 4 spaces, never tabs.
 
* '''Line length.''' Nothing is enforced. The median line is 36 characters and the 95th percentile is 86, but 7,927 lines pass 79 and 773 pass 120. Wrap new code at 100 characters.
 
* '''Line length.''' Nothing is enforced. The median line is 36 characters and the 95th percentile is 86, but 7,927 lines pass 79 and 773 pass 120. Wrap new code at 100 characters.
 
* '''Quotes.''' Double quotes outnumber single quotes roughly two to one. Prefer double quotes for new strings and keep the file's existing style for short dictionary keys.
 
* '''Quotes.''' Double quotes outnumber single quotes roughly two to one. Prefer double quotes for new strings and keep the file's existing style for short dictionary keys.

Latest revision as of 07:55, 12 November 2026

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 (zynthiangui'.py), engine widgets (zynthianwidget'.py), zynthianguiconfig.py and the main controller zynthian_gui.py
zyngine/ State manager, chain manager, processors, signal manager and one wrapper per synth engine (zynthianengine*.py)
zyngine/ctrldev/ MIDI controller drivers (zynthianctrldev*.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 (zynthianmain.py, zynthianheadless.py), the state schema (zynthianstateschema.py) and shell scripts

Where a new file goes:

  • A screen: zyngui/zynthiangui<name>.py, holding class zynthiangui<name>.
  • An engine widget: zyngui/zynthianwidget<name>.py.
  • A synth engine wrapper: zyngine/zynthianengine<name>.py.
  • A controller driver: zyngine/ctrldev/zynthianctrldev<vendor>_<model>.py.

1.3 Python file layout

Start every new module with the project banner. 163 of 185 files carry it; the shebang appears in 123 files and the coding line in 172. Keep both lines for consistency, although the coding line is redundant in Python 3. 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.

#!/usr/bin/python3
= -''- coding: utf-8 -''- =
= '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' =
= 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.

148 of the 155 zynthian_*.py modules in zyngui/ and zyngine/ define a class with the same name as the file, so a file and its class are easy to match. Follow that for new modules.

Several files are very large: zynthiangui.py (3,377 lines), zynthianstatemanager.py (3,022) and zynthiangui_mixer.py (2,927). You should put new features in a new module or helper class rather than growing these further. 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 uses snake case almost everywhere, including class names. Python follows this table; C and C++ are covered below it.

Item Convention Notes from the branch
Module files zynthian<area><name>.py, lower case Areas in use: gui (59 files), ctrldev (40), engine (20), widget (17). zynthianwidgetGxGraphicEQ.py mirrors a plugin name; avoid capitals in new names.
Project classes Snake case, same as the module (zynthianguibase) 157 classes start zynthian_; none use Zynthian.
Helper classes CamelCase (NotePlayer, StepSeqHandler) 78 helper classes use CamelCase; 10 use snake case, which you should avoid.
Functions and methods snake_case 4,438 definitions against 44 in camel case, almost all unittest hooks such as setUpClass.
Private members Leading underscore 318 private methods.
Constants UPPERCASE at module level (DRAGTHRESHOLD) 983 module-level constant assignments.
Event handlers on<event>, <name>cb or cb_<name> 180, 105 and 72 methods. All three exist, so follow the neighbouring methods in the file.
CUIA actions cuia_<action> 168 methods in zyngui/.
Signals S<AREA> and SS<AREA><EVENT> Defined in zynthiansignal_manager.py.
Environment variables ZYNTHIAN_<NAME> 128 distinct names are read by the Python code.

Choose descriptive names and avoid shadowing built-ins. getbgimg(self, id, width, height) in the mixer screen hides id; prefer imgid, chainid and similar.

In C and C++ under zynlibs/, classes are CamelCase (Pattern), methods are camelCase (258 definitions in .cpp files) and member variables take an m prefix, often followed by a type letter (mnBeats, 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. Nothing is enforced. The median line is 36 characters and the 95th percentile is 86, but 7,927 lines pass 79 and 773 pass 120. Wrap new code at 100 characters.
  • Quotes. Double quotes outnumber single quotes roughly two to one. Prefer double quotes for new strings and keep the file's existing style for short dictionary keys.
  • String building. Use f-strings (2,094 uses). .format() (302) and % (120) are older style.
  • Blank lines. Two between top-level definitions and one between methods, as PEP 8 says. About 200 top-level definitions in the branch have fewer.
  • Line endings and whitespace. Use LF endings and leave no trailing whitespace on lines you write. 231 lines have trailing whitespace and three files use CRLF.
  • Comparisons. You must compare with is None / is not None (841 uses against 17 for == None), and test booleans directly instead of == True.
  • Commented-out code. Delete it; git keeps the history. About 650 comment lines are disabled statements, for example the dead try block at the top of buildview() in zynthiangui_mixer.py.
  • Markers. Write TODO: or FIXME: with what is missing and why. The branch has 192 and 32 of them, many without context.
  • Mutable defaults and globals. You must not use [] or {} as a default argument (only 2 remain). Avoid global (77 uses); keep state on the instance.

An .editorconfig such as this one in your own checkout keeps most editors consistent. Do not enable trim-on-save for whole files, because that touches existing lines.

root = true

[*.{py,c,cpp,h}]
indent_style = space
indent_size = 4
end''of''line = lf
insert''final''newline = 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. runscript() in zynthiangui_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: dbtonorm() 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.formatexc() as zynthiansignal_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. runscript() in zynthiangui_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 zynthianguibase, directly or through the zynthianguiselector 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 sendqueued() or a handler registered with registerqueued() 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.afteridle(func, *args), as zynthiangui_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. runscript() in zynthiangui_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 zynthianguiconfig (colorbg, colorpanelbg, colortx, fontfamily, fontsize, 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 zynthiangui.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 queuethreadtask() 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. zynthiangui.py does this for switch events: zynswitchdefered() 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 aftercancel() in hide(). The branch has 40 after() calls but only 11 aftercancel() 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 initmidiproc() in zynthianctrldev_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 zynthianengine, call super().init(statemanager), then set self.name, self.nickname and self.jackname. Override only the hooks your engine needs. The common ones are getbanklist(), setbank(), getpresetlist(), setpreset(), getcontrollersdict(), sendcontrollervalue(), start() and stop().

  • Engines are registered in the dictionary in zynthianchainmanager.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.
  • zynthianenginesysex.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 zynthianctrldevbase, or one of its extended variants in the same folder, and name the file and class zynthianctrldev<vendor><model>. The default driver name is the class name minus its 17-character zynthianctrldev prefix. zynthianctrldev_duopiano.py (54 lines) is a minimal, readable model.

Class attribute Purpose
dev_ids Strings that identify the device when it is detected
drivername, driverdescription Optional display text
unroutefromchains 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
devzynpad, devzynmixer, dev_pated Capability flags
  • Do set-up in init() and clean-up in end(), and call super().end() from your override.
  • Handle input in midievent() and redraw device lights in refresh(). Save and restore driver state with getstate() and set_state().
  • Send output through libzyncore (for example devsendnoteon()), and use statemanager.addslowupdatecallback() for periodic work rather than a thread of your own.
  • Drivers are loaded and unloaded by zynthianctrldevmanager; never create them directly.

1.11.3 Widgets

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

  • Draw in refreshgui(). The base update() calls getmonitors() 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. zynthianstateschema.py documents them: it holds a commented example of the ZynthianState dictionary and its schemaversion (currently 6). The state manager, chain manager, chain, processor, controller, controller drivers and the zynseq wrapper each save with getstate() 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 zynthianstateschema.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 SNAPSHOTSCHEMAVERSION in zynthianlegacysnapshot.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. version5() 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 UPPERCASE name and a trailing comment with the unit and the reason, as mixer.c does for DPMRANGE_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 CMAKEBUILDTYPE 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 checkoutput(f"systemctl start {service}", shell=True) in zynthianstatemanager.py. Pass an argument list instead: checkoutput(["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(). zynthianstatemanager.py and zynthiangui.py both evaluate "zynthianguiconfig.{}".format(varname). Write getattr(zynthiangui_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: ZYNTHIANUIDIR, ZYNTHIANMYDATADIR, ZYNTHIANCONFIGDIR, ZYNTHIANDATA_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("ZYNTHIANX", default) (143 uses) rather than os.environ["ZYNTHIANX"] (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. zynthiangui.py calls the current screen's optional plotzctrls() 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/unittests.py orders its tests with names like testaa01_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 dbtonorm() are easy to test.
  • Run the suite with ./createvenv.sh once, then ./runtests.sh, before submitting any change to zynseq.
  • Try UI changes on a device or desktop install. source zynthiandebug.sh sets ZYNTHIANLOGLEVEL=10 for debug logging and ZYNTHIANUIPOWERSAVE_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/midibinding, 1822/faderlaw) 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 getstate() and setstate(), 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.