Repository navigation
Restructure modules into self-contained directories with per-device environment configs, config-driven dependency injection, src/ layout, dual-backend messaging service, and direct per-cycle loop() dispatch - #159
Conversation
…, and tests Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
|
@copilot pull the latest from develop, apply the restructure to the new modules. Resolve any merge conflicts. The config.yml files shouldn't need to specify the path of the python application anymore as it is the same directory as that file. Migrate the 'enabled' flag for each module out of the individual config.yml files and create a files that defines which modules are enabled so that they can be adjusted from a single file. Any modules that were original archived should remain in an archived folder. |
Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
Done in commit DiscordBot from develop: Added Path inference: Central Archived modules: The Additional fixes: Resolved key conflicts ( |
|
@copilot The next objective is to allow the same source to be configured for multiple devices / environments, namely:
Each device configuration should allow specific modules to be enabled / disabled, and any device specific config such as servo range values should be configured at this level. Any other configuration values should remain in the module config files. The startup and installer scripts already allow an 'environment' to be specified, and each config.yml can accept an optional 'environment' flag. Utilise and clean up this as part of the refactor. Create the environment config yaml files withing the modules directory where the 'enabled' yaml is current placed, and remove the enabled yaml file. Revisit the structure of the project, ensuring that it follows best practices and that all elements are in appropriately named directories that match the proposed restructure. Ensure all unit tests and documentation is updated to match this new approach, and that the autolaunch.sh accommodates specifying an environment to autolaunch. Also pull in the latest develop branch and handle merge conflicts |
|
@copilot The next objective is to allow the same source to be configured for multiple devices / environments, namely: Archie Each device configuration should allow specific modules to be enabled / disabled, and any device specific config such as servo range values should be configured at this level. Any other configuration values should remain in the module config files. The startup and installer scripts already allow an 'environment' to be specified, and each config.yml can accept an optional 'environment' flag. Utilise and clean up this as part of the refactor. Create the environment config yaml files withing the modules directory where the 'enabled' yaml is current placed, and remove the enabled yaml file. Revisit the structure of the project, ensuring that it follows best practices and that all elements are in appropriately named directories that match the proposed restructure. Ensure all unit tests and documentation is updated to match this new approach, and that the autolaunch.sh accommodates specifying an environment to autolaunch. Also pull in the latest develop branch and handle merge conflicts |
…ch.sh Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
Done in commit
Each environment file controls which modules are
Device-specific config moved to environment files — servo instances/poses, BNO055 I2C addresses, GPIO pin numbers, and display pin assignments were stripped from module
Develop changes applied — 237 tests passing (112 in
Addressed in commit |
Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
|
@copilot migrate any documentation from the docs directory to the appropriate module directory. Merge with module README if necessary. Move the environments config yaml directory out of modules and into the top level project directory. Update source to reference this correctly. Move the following directories into a new folder
The environments config yamls are not correctly configured. To understand which modules to enable (and the environment specific configuration, please reference the following branches and update accordingly:
|
…t; fix env configs Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
Done in commit Docs migrated to modules — Tests migrated to modules — All
Environment configs fixed based on branch references:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…m/makerforgetech/modular-biped into copilot/refactor-module-structure
|
@copilot Fix failing unit tests. Refactor messaging service to facilitate pypubsub (default) and mqtt messaging, by allowing messages to be sent by mqtt (by calling a different method in the base module class). Allow modules to subscribe to both services using the same method calls. The messaging service module config should allow the choice of (just) pypubsub, mqtt, or both. |
…aseModule, fix CI requirements Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
Done in commit CI test failures fixed — MessagingService refactored to support three protocol modes (configured via
# Send to all active backends (pubsub and/or mqtt)
self.publish('sensor/temperature', value=42.0)
# Send only to MQTT (e.g. for external system integration)
self.publish_mqtt('sensor/temperature', value=42.0)All 197 tests pass (33 in |
|
@copilot this implementation is flawed as each publish will be received twice, once for pubsub and once for MQTT. The default publish should be pubsub, unless otherwise specified. Make changes and ensure all tests pass. |
… protocol=both Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
Fixed in commit
When |
|
@copilot The system/loop event is called every cycle. This is unreliable with pubsub as the event does not always propagate as it should. Do not change the other loop events such as system/loop/1 etc. These must retain their behaviour. Ensure all tests pass. |
…ules Co-authored-by: danic85 <6583012+danic85@users.noreply.github.com>
Done in commit
Modules refactored:
|
…m/makerforgetech/modular-biped into copilot/refactor-module-structure
Replaces scattered config/docs/tests layout with co-located module directories, introduces per-device environment YAML files replacing a single
enabled.yml, moves all module-to-module dependency wiring out ofmain.pyinto the environment config, consolidates source code undersrc/, moves environment configs to the project root, refactors the messaging service to support pypubsub, MQTT, or both with clear per-method routing, and replaces the unreliablesystem/looppubsub event with a direct per-moduleloop()method call.What type of change is this?
📝 What does this change do?
Module structure — each module is fully self-contained under
src/modules/:src/directory —main.py,module_loader.py,system_loop.py, andmodules/are now co-located undersrc/.startup.shand CI both setPYTHONPATH=srcso all existing module imports continue to work unchanged.environments/at project root — moved out ofmodules/environments/to the top-levelenvironments/directory.ModuleLoaderaccepts anenvironments_folderparameter and auto-detects the project root viaos.path.dirnamerelative tosrc/module_loader.py.Per-device environment files (
environments/<env>.yml) replaceenabled.yml. Each file controls:enabledconfig:shallow-merged over module defaults)instances:replaces module list)inject:/on_inject:)Environment configs verified against per-device reference branches:
archie.yml— full biped: bus servos, IMU (head+body), vision, TFT display, braillespeak, buzzer, speechinput, serial, trackingbuddy.yml— PWM servos, NeoPixels (I2C/pin 12/count 12),gpio_motionpin 26, vision_imx500, tracking enabled; personality and chatgpt/telegram/tts disabled (matches buddy branch)cody.yml— I2C servos, BNO055 IMU with head+body instances, TFT display with full pin config, vision_imx500, ttsmodule with ElevenLabs config; personality injected with imu+vision (matches develop branch robot environment)server.yml— onlymessaging_service,logwrapper, anddiscordbotlaptop.yml— lean: core + chatgpt + discordbotConfig-driven dependency injection —
ModuleLoader.inject_dependencies()replaces all manual wiring inmain.py:Dual-backend MessagingService — refactored to support pypubsub, MQTT, or both, configured via
protocol:in the module config or environment override:protocolpubsub(default)mqttbothRouting is explicit to prevent double delivery:
subscribe(topic, callback)— registers on all active backends so callbacks receive messages from either pathpublish(topic, ...)— routes to the primary backend only: pypubsub whenprotocolispubsuborboth; MQTT whenprotocolismqttpublish_mqtt(topic, ...)— sends only to the MQTT backend; no-op if MQTT is not configuredBaseModule.publish_mqtt()— new method for modules that need to explicitly push data to the MQTT broker without triggering local pypubsub listeners:Direct per-cycle
loop()dispatch — replaces the unreliablesystem/looppubsub event with a direct method call pattern:BaseModulegains a no-oploop()placeholder — subclasses override it for per-cycle workSystemLoopnow iterates all active module instances and callsmodule.loop()directly each cycle instead of publishingsystem/loopSystemLoopsignature changed to(messaging_service, module_instances)so it receives the full loaded module dictsystem/loopare refactored to overrideloop():bus_servosubscribe('system/loop', self._process_queue)loop()override calls_process_queue()controller_handlersubscribe('system/loop', self._process_input)instart()loop()override calls_process_input()when runningvision/opencvsubscribe('system/loop', self.scan)loop()override callsscan()The timed events
system/loop/1,system/loop/10, andsystem/loop/60are unchanged — they continue to be published via pubsub.Docs migrated to modules —
docs/directory removed. All module documentation now lives in each module'sREADME.md. Infrastructure docs (BaseModule, ModuleLoader, SystemLoop) moved tosrc/docs/.ModuleLoader.mdrewritten to document the new environment-based architecture.Tests migrated to modules — all
tests/test_*.pymodule-specific tests replaced their less-comprehensive module-local stubs insrc/modules/*/tests/. Non-module tests (test_module_loader.py,test_base_module.py) moved tosrc/tests/. The oldtests/directory is removed.test.shand CI updated to usePYTHONPATH=srcwith discovery fromsrc/testsandsrc/modules.Module
config.ymlholds only class name, dependencies, and device-agnostic defaults. Hardware-specific values (servo IDs/ranges, GPIO pins, I2C addresses, display pins) live exclusively in the environment file.installers/autolaunch.sh— acceptsenable <environment>(e.g../autolaunch.sh enable buddy), validates the env file exists atenvironments/<env>.yml, and embeds it in the systemdExecStart.❓ Why is this change needed?
config/,docs/,tests/— hard to navigate per moduleenabled.ymlcould not express per-device differences (Archie has bus servos; a server has none)main.pymain.pygrew as an ad-hoc list of conditionals with no declarative structuremain.py,module_loader.py,system_loop.py,modules/) was mixed with project root files — no clearsrc/boundarymodules/rather than at the project root where they logically belongdocs/tree with no co-location with the modules it describedtests/duplicated and diverged from module-local testsMessagingServicecould only use one backend at a time, with no way to target MQTT explicitly from a module; the initial dual-backend implementation caused double delivery whenprotocol: bothbecausepublish()fanned out to all active backendssystem/looppubsub event was unreliable — pypubsub event propagation is not guaranteed to complete within a single cycle, causing missed or delayed per-cycle polling in modules such asbus_servo,controller_handler, andvision/opencv🛠️ How was this implemented?
src/layout:modules/,main.py,module_loader.py, andsystem_loop.pymoved intosrc/.startup.shupdated to referencesrc/main.py.test.shand.github/workflows/test.ymlupdated to usePYTHONPATH=srcwith two discovery passes (src/testsandsrc/modules).environments/at root:modules/environments/moved to top-levelenvironments/.ModuleLoader.__init__gains anenvironments_folderparameter; the default auto-detects the project root by going one level up fromsrc/module_loader.pyviaos.path.dirname(os.path.abspath(__file__)).Path inference:
ModuleLoaderderives the Python file from theconfig.ymllocation — the file must share its name with its containing directory (src/modules/gpio/motion/motion.py). Nopath:field needed.Environment loading:
_load_environment_config()readsenvironments/<env>.yml. Config values shallow-merge over module defaults;instances:fully replaces the module list.Dependency injection:
inject_dependencies(instances)readsinject:/on_inject:from the environment file after all modules are instantiated_resolve_inject_source()handles direct key lookup andPrefix_*wildcard (returns{inst.identifier: inst}dict)_set_attr()supports dotted-path notation (imu.head→target.imu['head'])MessagingService refactor:
MessagingService.__init__activatesPubSubMessagingServiceand/orMQTTMessagingServicebased on theprotocolconfig keysubscribe()registers the callback on all active backends (so it receives messages from either path)publish()routes to the primary backend only — pypubsub whenprotocolispubsuborboth; MQTT whenprotocolismqtt— preventing double delivery whenprotocol: bothpublish_mqtt()targets the MQTT backend exclusively regardless ofprotocolrequirements.txtupdated withpyyaml,requests,pypubsub, andpaho-mqttso CIpip install -r requirements.txtprovides the packages the test runner needsDirect
loop()dispatch:BaseModule.loop()added as a no-op placeholderSystemLoop.__init__acceptsmodule_instancesdict (replacing the singlepersonalityparameter) and stores a list of all module instancesSystemLoop.run()callsmodule.loop()on every module each cycle;messaging_service.publish('system/loop')removedbus_servo,controller_handler, andvision/opencveach overrideloop()and remove theirsystem/loopsubscriptionsDocs migration:
docs/directory deleted after verifying all module docs were present verbatim in moduleREADME.mdfiles. Fixed wrong title inbus_servo/README.md. Populated emptyneopixel/neopx/README.md. Infrastructure docs moved tosrc/docs/.Tests migration: Top-level
tests/test_*.pyfiles (which were more comprehensive) replaced module-local stubs insrc/modules/*/tests/.sys.pathmanipulation removed from migrated tests sincePYTHONPATH=srchandles resolution.tests/directory removed.Environment config reconciliation: Each environment YAML was verified against its reference branch config directory:
buddy.ymlreconciled against the buddy branch —personalitycorrected to disabled,vision_imx500andtrackingadded,oled_displayandinput_recorderremoved (absent from buddy branch)cody.ymlreconciled against the develop branch robot environment —bno055with head/body instances added,tft_displaywith full pin config added,vision_imx500added,ttsmoduleupdated with ElevenLabs service/voice config,gpio_motion/laser/oled_displayremoved (absent from develop config), personalityinjectupdated to wire imu.head, imu.body, and visionautolaunch.shupdated to validate and referenceenvironments/<env>.ymlat the project root.🧪 How was this tested?
src/tests/+ 167 insrc/modules/inject_dependencies,_resolve_inject_source,_set_attrcovering all injection patterns (simple, dict-path, wildcard, missing source,on_injectstring/list)pubsub,mqtt,both), primary-backend-only routing forpublish(), subscribe fan-out to all backends, andpublish_mqttisolation — including a dedicated test asserting MQTT is not called whenpublish()is invoked withprotocol: bothBaseModuletests extended to coverpublish_mqtt, and the new no-oploop()placeholderbus_servotests updated:test_loop_calls_process_queueassertsloop()drains the queue;system/loopsubscription asserted absentcontroller_handlertests updated:test_loop_calls_process_input_when_runningandtest_loop_does_not_call_process_input_when_not_runningcover the newloop()guardvision/opencvtests updated:test_loop_calls_scanassertsloop()triggers detection;setup_messagingasserted to make no subscriptionsbash test.shruns both discovery passes (src/testsandsrc/modules) cleanly withPYTHONPATH=src💥 Breaking changes
modules/enabled.ymlremoved — replaced byenvironments/<env>.ymlat the project root. Default environment isarchie.path:removed from module configs — Python filename must match its containing directory.Device-specific config moved — servo instances/poses, GPIO pins, I2C addresses, display pins no longer in module
config.yml; they belong in the appropriate environment file.Module key renames:
controller→controller_handler/xbox_controller;servos→bus_servo/servo;motion(laser) →laser;motion(read) →gpio_read.bus_servo/servo.pyrenamed tobus_servo/bus_servo.py.main.py,module_loader.py,system_loop.py,modules/moved tosrc/— any scripts or integrations referencing these at the project root must update their paths. UsePYTHONPATH=srcwhen invoking Python directly.modules/environments/moved toenvironments/— any external references to the old path must be updated.docs/removed — documentation now lives in each module'sREADME.mdand insrc/docs/.main.pywiring removed — manual injection blocks replaced byloader.inject_dependencies(). Any custom wiring must be declared in the environment YAML.MessagingServiceinternal structure changed — the old.messaging_serviceattribute (a single backend instance) is replaced by._pubsuband._mqtt.publish()no longer fans out to all backends; it routes to the primary backend only. Publicsubscribe(),publish(), andpublish_mqtt()are the supported interface.SystemLoopsignature changed — constructor now takes(messaging_service, module_instances)instead of(messaging_service, personality). Any code constructingSystemLoopdirectly must be updated.system/looppubsub event removed — modules must overrideBaseModule.loop()instead of subscribing tosystem/loop. The timed eventssystem/loop/1,system/loop/10, andsystem/loop/60are unchanged.requirements.txtnow contains packages — previously a no-op comment file; now installspyyaml,requests,pypubsub, andpaho-mqttfor CI and local test runs.🗂 Related issues
✅ PR Checklist
🚀 Thank you for your contribution to the project!
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.