diff --git a/.github/workflows/cmake-workflow.yml b/.github/workflows/cmake-workflow.yml index a86e183..0f0459b 100644 --- a/.github/workflows/cmake-workflow.yml +++ b/.github/workflows/cmake-workflow.yml @@ -165,7 +165,8 @@ jobs: cmake --build build -j"$(nproc)" # Stands in for an instrument's build: the core composed as a subproject, - # and a translation unit of the consumer's own that includes its headers + # a translation unit of the consumer's own that includes its headers, and + # the consumer's own daemon - name: Build the core as a subproject shell: bash run: | @@ -181,7 +182,10 @@ jobs: FetchContent_Declare(camerad SOURCE_DIR ${CAMERAD_SOURCE_DIR}) FetchContent_MakeAvailable(camerad) add_library(consumer OBJECT consumer.cpp) - target_link_libraries(consumer PRIVATE camerad_base) + target_link_libraries(consumer PRIVATE camerad::base) + add_executable(consumer_camerad ${CAMERAD_SOURCE_DIR}/camerad/archon_interface_factory.cpp) + target_link_libraries(consumer_camerad camerad::daemon camerad::archon) + set_target_properties(consumer_camerad PROPERTIES OUTPUT_NAME camerad) EOF cat > consumer.cpp <<'EOF' #include "camera_interface.h" diff --git a/CMakeLists.txt b/CMakeLists.txt index 58d714e..e6ad364 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,11 +88,29 @@ if (BUILD_PYTHON_MODULE) set(CMAKE_POSITION_INDEPENDENT_CODE ON) endif() +# The core's own module is tested by importing it, so that ctest from the +# build directory finds that test +if (BUILD_PYTHON_MODULE AND PROJECT_IS_TOP_LEVEL) + enable_testing() +endif() + # A project that composes this one gets the libraries; the programs and the # install rules are built only if it asks for them option(CAMERAD_BUILD_TOOLS "Build the emulator, listener, socksend and shm_reader" ${PROJECT_IS_TOP_LEVEL}) option(CAMERAD_INSTALL "Generate install rules" ${PROJECT_IS_TOP_LEVEL}) +# This lets an instrument package share an environment with the generic package +# and other instruments. The module installs into a Python package of this name, +# and camerad and the emulator are named for it. The name is supplied by +# whoever builds the instrument package. +# +# This is temporary and I'll remove it once each instrument's +# repository declares its own module and writes its own install rules +set(CAMERAD_PYTHON_PACKAGE "" CACHE STRING "Python package an instrument package installs its module into") +if (CAMERAD_PYTHON_PACKAGE) + string(REPLACE "_" "-" CAMERAD_PACKAGE_SUFFIX ${CAMERAD_PYTHON_PACKAGE}) +endif() + add_subdirectory(${PROJECT_BASE_DIR}/utils) add_subdirectory(${PROJECT_BASE_DIR}/common) add_subdirectory(${PROJECT_BASE_DIR}/camerad) diff --git a/README.md b/README.md index 372c82e..78122c9 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,44 @@ If you encounter any problems or have questions about this project, please open Asserts every expected keyword is present and carries the value the emulator's `MODE_DEFAULT` implies, so a keyword that stops being populated fails rather than going unnoticed. Needs no FITS library. Both emulator CI jobs run it after their exposure. +### Composing the core + +An instrument's build composes the core as a subproject (CMake `FetchContent`) and declares its own `camerad` and Python module. The core declares those two only when it is the top-level project. Link only the `camerad::` names: + +- `camerad::base` — the `Interface` base class and image processing +- `camerad::archon` — the controller library, named after `CONTROLLER` +- `camerad::daemon` — the daemon's `main` and its server +- `camerad::python` — the module's source, compiled inside the target that links it + +```cmake +set(CONTROLLER archon CACHE STRING "") +set(BUILD_PYTHON_MODULE ON CACHE BOOL "") +FetchContent_Declare(camerad GIT_REPOSITORY GIT_TAG ) +FetchContent_MakeAvailable(camerad) + +add_executable(myinst_camerad myinst_interface_factory.cpp myinst_instrument.cpp) +target_link_libraries(myinst_camerad camerad::daemon camerad::archon) +set_target_properties(myinst_camerad PROPERTIES OUTPUT_NAME camerad) + +find_package(Python3 COMPONENTS Interpreter Development REQUIRED) +# pybind11 must come from the interpreter found above. A pip-installed one is +# not on CMake's search path, and CMake may silently find a system copy +# instead: pass -Dpybind11_DIR=$( -m pybind11 --cmakedir), or +# ask ${Python3_EXECUTABLE} as python/CMakeLists.txt does +find_package(pybind11 CONFIG REQUIRED) +pybind11_add_module(camera_interface myinst_interface_factory.cpp myinst_instrument.cpp) +target_link_libraries(camera_interface PRIVATE camerad::python camerad::archon) +target_compile_definitions(camera_interface PRIVATE CAMERAD_INSTRUMENT_NAME="myinst") +``` + +- Each final target compiles exactly one interface factory definition. The linker rejects zero or two for the daemon. A module with none still links and fails only when imported, so the instrument's tests import it and check `instrument_name()`. +- `BUILD_PYTHON_MODULE=ON` is needed for a module: it declares `camerad::python` and makes the core's static libraries position-independent. +- The module's file name is `camera_interface`, the import name fixed in its source. +- Without `CAMERAD_INSTRUMENT_NAME` the module reports `"none"`. +- Finding Python and pybind11 is the consumer's job; the core does neither as a subproject. `python/CMakeLists.txt` shows one way. + +An instrument repository that installs its module puts it inside a Python package of its own, e.g. `myinst/camera_interface…so` with an `__init__.py` that re-exports it, so the module keeps its name and both `import myinst` and `import myinst.camera_interface` work. It names its daemon and emulator for the package, e.g. `camerad-myinst` and `camerad-emulator-myinst`, so that installing it never replaces another package's files. + ## Installing with pip `pip install` builds the same artifacts and places them in the target environment, so `import camera_interface` needs no `PYTHONPATH` and `camerad` is on `PATH` whenever the environment is active: @@ -183,17 +221,26 @@ assert camera_interface.instrument_name() == "hispec_tracking_camera" A compiler and the full dependency set have to be present wherever `pip install` runs, since it compiles camerad and the module from source. -### Per-instrument packages +### Instrument packages -Installing this way twice replaces the first build, since both wheels are called `camera-interface` and both modules `camera_interface`. `packaging/` holds one directory per instrument that needs its own package, each fixing the instrument and the module name so neither is the caller's to pass: +An instrument package can share an environment with the generic package and with other instruments. Its build sets `CAMERAD_PYTHON_PACKAGE` to the package's name, e.g. in a `pyproject.toml` in the instrument's own repository that builds the core from its checkout: -```bash -$ pip install ./camera-interface/packaging/tracking +```toml +[tool.scikit-build] +cmake.source-dir = "external/camera-interface" +wheel.packages = [] +cmake.define.BUILD_PYTHON_MODULE = "ON" +cmake.define.CONTROLLER = "archon" +cmake.define.INSTRUMENT = "myinst" +cmake.define.CAMERAD_PYTHON_PACKAGE = "myinst_camera" ``` -That installs `camera-interface-tracking`, providing the module `camera_interface_tracking`. Such packages are independent of each other and of the plain `camera-interface` above, so any combination can share one environment. +That package installs: -To add one, copy a `packaging/*/pyproject.toml` and change `name`, `INSTRUMENT` and `CAMERAD_MODULE_NAME`. +- `myinst_camera/camera_interface…so` and `myinst_camera/__init__.py`, imported as `import myinst_camera` or `import myinst_camera.camera_interface`; +- `camerad-myinst-camera` and `camerad-emulator-myinst-camera`, named for the package with `_` replaced by `-`. + +It does not install `camerad-socksend` or `camerad-shm-reader`, which come with the generic package. The generic package is unchanged: `import camera_interface`, plus `camerad`, `camerad-emulator` and `camerad-socksend`. Uninstalling one package leaves the others' files in place. ## Python Module @@ -218,14 +265,7 @@ Every command `camerad` accepts is reachable: the base commands are bound as met The controller and instrument are fixed at CMake configure time, so `instrument_name()` and `controller_name()` report which build was loaded. A failed command raises `RuntimeError`. -`-DCAMERAD_MODULE_NAME=` renames the module and its file together, so per-instrument builds can be imported side by side. [Per-instrument packages](#per-instrument-packages) is the packaged form of the same thing: - -```bash -$ cmake -DBUILD_PYTHON_MODULE=ON -DINSTRUMENT=hispec_tracking_camera \ - -DCAMERAD_MODULE_NAME=camera_interface_tracking .. -``` - -It defaults to `camera_interface`, so a build that does not set it is unaffected. +Every build's module is named `camera_interface`. The generic build installs it at the top level, and an instrument package installs it inside a package of its own ([Instrument packages](#instrument-packages)), so several instruments can share one environment. Importing several instruments into one process is intended to be supported but is not yet tested; for now, use one instrument per process. `output_status()` is a snapshot, never a barrier: the FITS writer queues and drops frames by design because disk is slower than acquisition can be, so nothing here lets a caller stall acquisition by waiting on an output. Anything needing to be woken per frame should attach to the shared-memory segment, which posts semaphores. @@ -344,4 +384,4 @@ Sensor `C` is available only on **HeaterX** modules. --- David Hale - \ No newline at end of file + diff --git a/camerad/CMakeLists.txt b/camerad/CMakeLists.txt index 2837294..6bac0fc 100644 --- a/camerad/CMakeLists.txt +++ b/camerad/CMakeLists.txt @@ -27,6 +27,7 @@ option(CONTROLLER "Choose controller type { bob joe archon astrocam }") if (CONTROLLER STREQUAL "bob") message (STATUS "controller: Bob") add_definitions(-DCONTROLLER_BOB) + set (INTERFACE_DEFINITIONS CONTROLLER_BOB) set (INTERFACE_SOURCES ${CAMERAD_DIR}/bob_interface.cpp ) @@ -36,6 +37,7 @@ if (CONTROLLER STREQUAL "bob") elseif (CONTROLLER STREQUAL "joe") message (STATUS "controller: Joe") add_definitions(-DCONTROLLER_JOE) + set (INTERFACE_DEFINITIONS CONTROLLER_JOE) set (INTERFACE_SOURCES ${CAMERAD_DIR}/joe_interface.cpp ) @@ -45,18 +47,23 @@ elseif (CONTROLLER STREQUAL "joe") elseif (CONTROLLER STREQUAL "archon") message (STATUS "controller: Archon") add_definitions(-DCONTROLLER_ARCHON) + set (INTERFACE_DEFINITIONS CONTROLLER_ARCHON) set (INTERFACE_TARGET archon) set (INTERFACE_SOURCES ${CAMERAD_DIR}/archon_interface.cpp ${CAMERAD_DIR}/archon_controller.cpp ${CAMERAD_DIR}/archon_exposure_modes.cpp ) + set (INTERFACE_LIBS + network + ) # ---------------------------------------------------------------------------- # AstroCam ARC-64/66 PCI/e # ---------------------------------------------------------------------------- elseif (CONTROLLER STREQUAL "astrocam") message (STATUS "controller: AstroCam GenIII PCI/PCIe") add_definitions (-DCONTROLLER_ASTROCAM) + set (INTERFACE_DEFINITIONS CONTROLLER_ASTROCAM) set (INTERFACE_TARGET astrocam) set (ARCAPI_DIR "${PROJECT_BASE_DIR}/ARC") find_path (ARCAPI_BASE "CArcBase.h" PATHS ${ARCAPI_DIR}/CArcBase/inc) @@ -149,14 +156,19 @@ target_link_libraries(camerad_base PUBLIC # The public headers need C++17. The standard set for this directory is not # inherited by a project consuming this one, so it has to travel with the target target_compile_features(camerad_base PUBLIC cxx_std_17) +add_library(camerad::base ALIAS camerad_base) # ---------------------------------------------------------------------------- # controller implementation, linking the base PUBLIC so that anything # depending on the controller gets the base transitively # ---------------------------------------------------------------------------- add_library(${INTERFACE_TARGET} ${INTERFACE_SOURCES}) -target_link_libraries(${INTERFACE_TARGET} PUBLIC camerad_base) +target_link_libraries(${INTERFACE_TARGET} PUBLIC camerad_base ${INTERFACE_LIBS}) target_include_directories(${INTERFACE_TARGET} PUBLIC ${INTERFACE_INCLUDES}) +# so that code compiled against the controller, such as a consumer's Python +# module, knows which controller it is +target_compile_definitions(${INTERFACE_TARGET} INTERFACE ${INTERFACE_DEFINITIONS}) +add_library(camerad::${INTERFACE_TARGET} ALIAS ${INTERFACE_TARGET}) # ---------------------------------------------------------------------------- # instrument implementation, if any. @@ -192,44 +204,58 @@ include_directories(${BOOST_INCLUDES}) find_package(Threads) # ---------------------------------------------------------------------------- -# build the camera daemon +# the camera daemon's main and its Server. No controller and no interface +# factory definition: the final target supplies both # ---------------------------------------------------------------------------- -add_executable(camerad +add_library(camerad_daemon STATIC ${CAMERAD_DIR}/camerad.cpp ${CAMERAD_DIR}/camera_server.cpp - ${INSTRUMENT_REGISTRATION} - ${INSTRUMENT_OBJECTS} ) - -# ---------------------------------------------------------------------------- -# link everything -# ---------------------------------------------------------------------------- -target_link_libraries(camerad +target_link_libraries(camerad_daemon PUBLIC + camerad_base network utilities logentry - ${INTERFACE_TARGET} - ${INTERFACE_LIBS} ${CMAKE_THREAD_LIBS_INIT} - ${CCFITS_LIB} - ${CFITS_LIB} - ${BOOST_INCLUDES} - Boost::thread - Boost::chrono - ${ZMQPP_LIB} - ${ZMQ_LIB} ) - -if (CAMERAD_INSTALL) - install(TARGETS camerad RUNTIME DESTINATION ${CAMERAD_INSTALL_BINDIR}) -endif() +add_library(camerad::daemon ALIAS camerad_daemon) # ---------------------------------------------------------------------------- -# optional Python module +# the Python module's source, compiled inside each target that links this, +# so that the module is built for that target's instrument. Only with +# BUILD_PYTHON_MODULE, which is what makes the static libraries PIC # ---------------------------------------------------------------------------- -# Added here, not at the top level, to inherit the controller and instrument -# selection above. The option itself is declared at the top level if (BUILD_PYTHON_MODULE) - add_subdirectory(${PROJECT_BASE_DIR}/python ${CMAKE_BINARY_DIR}/python) + add_library(camerad_python INTERFACE) + target_sources(camerad_python INTERFACE ${PROJECT_BASE_DIR}/python/camera_interface_module.cpp) + target_link_libraries(camerad_python INTERFACE camerad_base) + add_library(camerad::python ALIAS camerad_python) endif() +# ---------------------------------------------------------------------------- +# final targets, only when this is the top-level project. A project that +# composes this one declares its own, linking the libraries above +# ---------------------------------------------------------------------------- +if (PROJECT_IS_TOP_LEVEL) + add_executable(camerad + ${INSTRUMENT_REGISTRATION} + ${INSTRUMENT_OBJECTS} + ) + target_link_libraries(camerad + camerad_daemon + ${INTERFACE_TARGET} + ) + if (CAMERAD_PYTHON_PACKAGE) + set_target_properties(camerad PROPERTIES OUTPUT_NAME camerad-${CAMERAD_PACKAGE_SUFFIX}) + endif() + + if (CAMERAD_INSTALL) + install(TARGETS camerad RUNTIME DESTINATION ${CAMERAD_INSTALL_BINDIR}) + endif() + + # Added here, not at the top level, to inherit the controller and instrument + # selection above. The option itself is declared at the top level + if (BUILD_PYTHON_MODULE) + add_subdirectory(${PROJECT_BASE_DIR}/python ${CMAKE_BINARY_DIR}/python) + endif() +endif() diff --git a/emulator/CMakeLists.txt b/emulator/CMakeLists.txt index 18235f6..e796725 100644 --- a/emulator/CMakeLists.txt +++ b/emulator/CMakeLists.txt @@ -50,7 +50,11 @@ target_include_directories(emulator PRIVATE ${CFITSIO_INCLUDE_DIRS}) target_link_directories(emulator PRIVATE ${CFITSIO_LIBRARY_DIRS}) # Prefixed because "emulator" is too generic for a shared bin directory -set_target_properties(emulator PROPERTIES OUTPUT_NAME camerad-emulator) +if( CAMERAD_PYTHON_PACKAGE ) + set_target_properties(emulator PROPERTIES OUTPUT_NAME camerad-emulator-${CAMERAD_PACKAGE_SUFFIX}) +else() + set_target_properties(emulator PROPERTIES OUTPUT_NAME camerad-emulator) +endif() if( CAMERAD_INSTALL ) install(TARGETS emulator RUNTIME DESTINATION ${CAMERAD_INSTALL_BINDIR}) diff --git a/packaging/tracking/pyproject.toml b/packaging/tracking/pyproject.toml index e6f07e4..c770646 100644 --- a/packaging/tracking/pyproject.toml +++ b/packaging/tracking/pyproject.toml @@ -16,4 +16,3 @@ wheel.packages = [] cmake.define.BUILD_PYTHON_MODULE = "ON" cmake.define.CONTROLLER = "archon" cmake.define.INSTRUMENT = "hispec_tracking_camera" -cmake.define.CAMERAD_MODULE_NAME = "camera_interface_tracking" diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 7819807..968b9dd 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -28,7 +28,6 @@ endif() find_package(pybind11 CONFIG REQUIRED) pybind11_add_module(camera_interface - ${PROJECT_BASE_DIR}/python/camera_interface_module.cpp ${INSTRUMENT_REGISTRATION} ${INSTRUMENT_OBJECTS} ) @@ -41,33 +40,49 @@ target_include_directories(camera_interface PRIVATE ${INTERFACE_INCLUDES} ) -set(CAMERAD_MODULE_NAME "camera_interface" CACHE STRING - "Import name of the Python module") - -# Filename and import name must agree, so both come from CAMERAD_MODULE_NAME -set_target_properties(camera_interface PROPERTIES OUTPUT_NAME ${CAMERAD_MODULE_NAME}) - -target_compile_definitions(camera_interface PRIVATE - CAMERAD_INSTRUMENT_NAME="${INSTRUMENT}" - CAMERAD_MODULE_NAME=${CAMERAD_MODULE_NAME} -) +# Without an instrument the module source reports "none" +if (INSTRUMENT) + target_compile_definitions(camera_interface PRIVATE + CAMERAD_INSTRUMENT_NAME="${INSTRUMENT}" + ) +endif() -# Same link set as camerad, minus the server sources it does not use +# camerad_python carries the module source, so it compiles here, with this +# target's instrument name target_link_libraries(camera_interface PRIVATE - network - utilities - logentry + camerad_python ${INTERFACE_TARGET} - ${INTERFACE_LIBS} - ${CMAKE_THREAD_LIBS_INIT} - ${CCFITS_LIB} - ${CFITS_LIB} - Boost::thread - Boost::chrono - ${ZMQPP_LIB} - ${ZMQ_LIB} ) if (CAMERAD_INSTALL) - install(TARGETS camera_interface LIBRARY DESTINATION ${CAMERAD_INSTALL_LIBDIR}) + if (CAMERAD_PYTHON_PACKAGE) + # The module keeps its name inside the package; the __init__.py lets + # "import " behave as the module too + file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/package/__init__.py [[ +from . import camera_interface as _module +from .camera_interface import * # noqa: F401,F403 + +__all__ = [name for name in dir(_module) if not name.startswith("_")] +]]) + install(TARGETS camera_interface + LIBRARY DESTINATION ${CAMERAD_INSTALL_LIBDIR}/${CAMERAD_PYTHON_PACKAGE}) + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/package/__init__.py + DESTINATION ${CAMERAD_INSTALL_LIBDIR}/${CAMERAD_PYTHON_PACKAGE}) + else() + install(TARGETS camera_interface LIBRARY DESTINATION ${CAMERAD_INSTALL_LIBDIR}) + endif() +endif() + +# A shared library may leave symbols undefined, so a module missing its +# interface factory definition links and fails only when imported +if (INSTRUMENT) + set(CAMERAD_EXPECTED_INSTRUMENT ${INSTRUMENT}) +else() + set(CAMERAD_EXPECTED_INSTRUMENT none) endif() +add_test(NAME python_import + COMMAND ${Python3_EXECUTABLE} -c "import camera_interface\nassert camera_interface.instrument_name() == '${CAMERAD_EXPECTED_INSTRUMENT}', camera_interface.instrument_name()\nassert camera_interface.controller_name() == '${CONTROLLER}', camera_interface.controller_name()" +) +set_tests_properties(python_import PROPERTIES + ENVIRONMENT PYTHONPATH=$ +) diff --git a/python/camera_interface_module.cpp b/python/camera_interface_module.cpp index 94d9788..06a7fd9 100644 --- a/python/camera_interface_module.cpp +++ b/python/camera_interface_module.cpp @@ -118,11 +118,7 @@ namespace { } -#ifndef CAMERAD_MODULE_NAME -#define CAMERAD_MODULE_NAME camera_interface -#endif - -PYBIND11_MODULE(CAMERAD_MODULE_NAME, module) { +PYBIND11_MODULE(camera_interface, module) { module.doc() = "Direct control of a camera-interface camera, without camerad"; module.def("instrument_name", [] { return std::string(INSTRUMENT_NAME); }, @@ -130,9 +126,7 @@ PYBIND11_MODULE(CAMERAD_MODULE_NAME, module) { module.def("controller_name", [] { return std::string(CONTROLLER_NAME); }, "Return the controller this module was built for"); - // module_local keeps this out of pybind11's process-wide type registry, so - // two per-instrument builds can be imported together - py::class_(module, "Camera", py::module_local(), + py::class_(module, "Camera", "One camera, configured from a camerad .cfg file.\n\n" "Construction performs the same setup camerad does at startup: read the\n" "config, initialize logging, then configure the controller, interface,\n" diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index fe1ba83..d5dd7bc 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -80,7 +80,8 @@ if(CAMERAD_BUILD_TOOLS) set_target_properties(socksend PROPERTIES OUTPUT_NAME camerad-socksend) endif() -if(CAMERAD_BUILD_TOOLS AND CAMERAD_INSTALL) +# An instrument package leaves the generic tools to the generic package +if(CAMERAD_BUILD_TOOLS AND CAMERAD_INSTALL AND NOT CAMERAD_PYTHON_PACKAGE) install(TARGETS socksend RUNTIME DESTINATION ${CAMERAD_INSTALL_BINDIR}) if (ENABLE_SHM_OUTPUT) install(TARGETS shm_reader RUNTIME DESTINATION ${CAMERAD_INSTALL_BINDIR})