diff --git a/.github/workflows/emulator-integration.yml b/.github/workflows/emulator-integration.yml index f2ace2d..a82c142 100644 --- a/.github/workflows/emulator-integration.yml +++ b/.github/workflows/emulator-integration.yml @@ -148,11 +148,12 @@ jobs: echo "$resp" | grep -q "$2" || { echo "expected \"$2\""; exit 1; } } - expect "open" "DONE" - expect "load" "DONE" - expect "power on" "DONE" - expect "exptime 0.1" "DONE" - expect "expose 1" "DONE" + expect "open" "DONE" + expect "load" "DONE" + expect "power on" "DONE" + expect "exposuremode SINGLE" "^SINGLE DONE" + expect "exptime 0.1" "DONE" + expect "expose 1" "DONE" # instrument command, replying with the value configure_instrument() read expect "refvalue" "^reference-cfg-value DONE" @@ -163,8 +164,25 @@ jobs: expect "exposuremode REFERENCE" "^REFERENCE DONE" expect "expose 1" "DONE" expect "exposuremode" "^REFERENCE DONE" + + # applying an ACF mode section leaves the exposure mode alone + expect "acfmode DEFAULT" "^DONE" + expect "exposuremode" "^REFERENCE" + expect "exposuremode SINGLE" "^SINGLE DONE" expect "expose 1" "DONE" + # acfmode and getp are the Archon interface's own commands, reached + # through controller_cmd; a name nobody claims is an error + expect "acfmode DEFAULT" "^DONE" + expect "getp nRowsQuad" "^[0-9]" + expect "nosuchcommand" "^not_supported ERROR" + + # a load leaves no exposure mode selected, so an exposure is refused + expect "load" "DONE" + expect "power on" "DONE" + expect "exposuremode" "^undefined ERROR" + expect "expose 1" "^exposure mode not set" + - name: Reference module import test run: ctest --test-dir build-reference --output-on-failure diff --git a/README.md b/README.md index 78122c9..3576502 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,7 @@ camera.instrument_cmd("roi", "51 60 51 60") print(camera.output_status()) # frames written, dropped, last file ``` -Every command `camerad` accepts is reachable: the base commands are bound as methods, instrument-specific ones go through `instrument_cmd()` (enumerable with `instrument_commands()`), and controller-specific ones such as `mode`, `raw`, `readacf`, `loadtiming`, `heater` and `sensor` go through `controller_cmd()`. Only `exit` is omitted, since the process belongs to the caller. +Every command `camerad` accepts is reachable: the base commands are bound as methods, instrument-specific ones go through `instrument_cmd()` (enumerable with `instrument_commands()`), and controller-specific ones such as `raw`, `readacf`, `loadtiming`, `heater` and `sensor` go through `controller_cmd()`, which forwards any other name to the instrument as `camerad` does. Only `exit` is omitted, since the process belongs to the caller. 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`. diff --git a/camerad/archon_interface.cpp b/camerad/archon_interface.cpp index ea4d818..b8441ea 100644 --- a/camerad/archon_interface.cpp +++ b/camerad/archon_interface.cpp @@ -69,10 +69,9 @@ namespace Camera { /***** Camera::ArchonInterface::controller_cmd ******************************/ /** * @brief dispatcher for Archon-specific commands - * @details This allows dispatching Archon controller specific commands by - * receiving the commands and args and calling the appropriate - * controller-specific function. This should be calling - * ArchonInterface functions. + * @details The server calls this for every name the core does not + * recognize centrally. It handles the commands unique to the + * Archon and passes any other name to instrument_cmd. * @param[in] cmd command * @param[in] args argument list * @param[out] retstring return string @@ -102,8 +101,8 @@ namespace Camera { return this->set_parameter(args, retstring); } else - if ( cmd == CAMERAD_MODE ) { - return this->set_camera_mode(args, retstring); + if ( cmd == CAMERAD_ACFMODE ) { + return this->acfmode(args, retstring); } else if ( cmd == CAMERAD_RAW ) { @@ -122,8 +121,7 @@ namespace Camera { return this->sensor(args, retstring); } else { - retstring="unrecognized command"; - return ERROR; + return this->instrument_cmd(cmd, args, retstring); } } /***** Camera::ArchonInterface::controller_cmd ******************************/ @@ -722,6 +720,10 @@ namespace Camera { const std::string function("Camera::ArchonInterface::load_firmware"); logwrite(function, acffile); + // clear the exposure mode, so that none is selected after a load + // + this->exposuremode.reset(); + // load the ACF file and write to Archon configuration memory // long error = this->controller->load_acf(acffile); @@ -735,7 +737,7 @@ namespace Camera { // if (error != NO_ERROR) this->controller->fetchlog(); - // set the mode to DEFAULT + // set the acfmode to DEFAULT // if (error == NO_ERROR) error = this->set_camera_mode(std::string("DEFAULT")); @@ -802,27 +804,24 @@ namespace Camera { /***** Camera::ArchonInterface::load_timing *********************************/ - /***** Camera::ArchonInterface::set_camera_mode *****************************/ + /***** Camera::ArchonInterface::acfmode *************************************/ /** - * @brief set a camera "mode" - * @details Allowable camera modes are defined in the ACF file and specify - * a set of camera settings which can be used to change camera - * settings. - * This version fits the general description of all virtual functions - * inherited by the Camera::Interface base class and is overridden - * by an Archon-specific implementation. + * @brief applies an ACF mode section directly + * @details Applies the settings in the ACF file's [MODE_] section. + * It does not select an exposure mode. * @param[in] args requested mode name (case-insensitive) * @param[out retstring current mode name * @return ERROR|NO_ERROR * */ - long ArchonInterface::set_camera_mode(std::string args, std::string &retstring) { - const std::string function("Camera::ArchonInterface::set_camera_mode"); + long ArchonInterface::acfmode(const std::string &args, std::string &retstring) { + const std::string function("Camera::ArchonInterface::acfmode"); // Help if (args=="?" || args=="help") { - retstring = CAMERAD_MODE; + retstring = CAMERAD_ACFMODE; retstring.append( " \n" ); - retstring.append( " Applies camera settings associated with MODE_ specified in the ACF file.\n"); + retstring.append( " Applies the settings of [MODE_] in the ACF file.\n" ); + retstring.append( " Does not select an exposure mode.\n" ); retstring.append( " Valid modes are: {" ); for (const auto &mode : this->controller->modemap) { retstring.append(" "); retstring.append(mode.first); @@ -833,6 +832,9 @@ namespace Camera { // call the work function return( this->set_camera_mode(args) ); } + /***** Camera::ArchonInterface::acfmode *************************************/ + + /***** Camera::ArchonInterface::set_camera_mode *****************************/ /** * @brief set a camera "mode" @@ -876,12 +878,6 @@ namespace Camera { // if we made it all the way to the end then this is the selected mode this->controller->selectedmode = modeselect; - std::string target = this->default_exposure_mode_name(); - for (const auto &m : this->get_exposure_modes()) { - if (m == modeselect) { target = modeselect; break; } - } - this->set_exposure_mode(target, {}); - return NO_ERROR; } /***** Camera::ArchonInterface::set_camera_mode *****************************/ diff --git a/camerad/archon_interface.h b/camerad/archon_interface.h index dff5231..89e7caa 100644 --- a/camerad/archon_interface.h +++ b/camerad/archon_interface.h @@ -48,11 +48,13 @@ namespace Camera { long do_expose() override; - // Archon controller command dispatcher + // Archon controller command dispatcher. The server calls it for every + // name the core does not recognize centrally; it handles the Archon + // commands and passes any other name to instrument_cmd // long controller_cmd(const std::string &cmd, const std::string &args, - std::string &retstring) override; + std::string &retstring) final; // These functions are specific to the Archon Interface and are not // found in the base class. @@ -64,15 +66,12 @@ namespace Camera { long load_timing(std::string cmd, std::string &reply); long read_acf(const std::string &filename); long set_parameter(const std::string &args, std::string &retstring); - long set_camera_mode(std::string args, std::string &retstring); long set_camera_mode(std::string modeselect); long set_vcpu_inreg(const std::string &args, std::string &retstring); long heater(const std::string &args, std::string &retstring); long sensor(const std::string &args, std::string &retstring); long autofetch_mode(const std::string &args, std::string &retstring); - - // Fallback for set_camera_mode when the camera-mode name is unknown - virtual std::string default_exposure_mode_name() const { return "SINGLE"; } + long acfmode(const std::string &args, std::string &retstring); char* get_framebuf() { return controller->framebuf; } diff --git a/camerad/camera_interface.h b/camerad/camera_interface.h index 95d7d86..30cdc18 100644 --- a/camerad/camera_interface.h +++ b/camerad/camera_interface.h @@ -128,30 +128,32 @@ namespace Camera { virtual long do_expose() = 0; - /** @brief returns true if cmd is an instrument-specific command - */ - virtual bool is_instrument_command(const std::string &cmd) { return false; } - // Names this instrument handles, so a caller can enumerate rather than // guess. CAMERAD_SYNTAX lists none of them virtual std::vector instrument_commands() const { return {}; } /** @brief returns error if not overridden + * @details The end of the dispatch chain. A name that reaches this + * default is claimed by no handler. */ virtual long instrument_cmd(const std::string &cmd, const std::string &args, std::string &retstring) { + const std::string function("Camera::Interface::instrument_cmd"); + logwrite(function, "ERROR unknown command: "+cmd); retstring="not_supported"; return ERROR; } - /** @brief returns error if not overridden + /** @brief handles commands unique to the controller + * @details The server calls this for every name the core does not + * recognize centrally. A controller interface handles its own + * commands and passes any other name to instrument_cmd. */ virtual long controller_cmd(const std::string &cmd, const std::string &args, std::string &retstring) { - retstring="not_supported"; - return ERROR; + return this->instrument_cmd(cmd, args, retstring); } }; diff --git a/camerad/camera_server.cpp b/camerad/camera_server.cpp index cdfe941..8341ee7 100644 --- a/camerad/camera_server.cpp +++ b/camerad/camera_server.cpp @@ -226,14 +226,6 @@ namespace Camera { } ret = HELP; } - /** - * instrument-specific commands - */ - else - if (interface->is_instrument_command(cmd)) - { - ret = interface->instrument_cmd(cmd, args, retstring); - } else if ( cmd == CAMERAD_ABORT ) { ret = interface->abort(args, retstring); @@ -252,7 +244,7 @@ namespace Camera { } else if ( cmd == CAMERAD_BIN ) { - ret = interface->bias(args, retstring); + ret = interface->bin(args, retstring); } else if ( cmd == CAMERAD_CLOSE ) { @@ -304,60 +296,11 @@ namespace Camera { ret = interface->datacube(args, retstring); } - /** - * controller-specific commands - */ - else - if ( cmd == CAMERAD_LOADTIMING ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == CAMERAD_READACF ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == "getp" ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == "inreg" ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == "setp" ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == CAMERAD_MODE ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == CAMERAD_RAW ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == "bob" ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == "autofetch_mode" ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == CAMERAD_HEATER ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - else - if ( cmd == CAMERAD_SENSOR ) { - ret = interface->controller_cmd(cmd, args, retstring); - } - - // unknown commands generate an error + // any other name goes to the controller interface, which forwards + // what it does not handle to the instrument // else { - message.str(""); message << "ERROR unknown command: " << cmd; - logwrite(function, message.str()); - ret = ERROR; + ret = interface->controller_cmd(cmd, args, retstring); } // If retstring not empty then append "DONE" or "ERROR" depending on value of ret, diff --git a/common/camerad_commands.h b/common/camerad_commands.h index 639358b..124b966 100644 --- a/common/camerad_commands.h +++ b/common/camerad_commands.h @@ -9,6 +9,7 @@ #pragma once const std::string CAMERAD_ABORT("abort"); +const std::string CAMERAD_ACFMODE("acfmode"); const std::string CAMERAD_AUTODIR("autodir"); const std::string CAMERAD_BASENAME("basename"); const std::string CAMERAD_BIAS("bias"); @@ -39,7 +40,6 @@ const std::string CAMERAD_LOADTIMING("loadtiming"); const std::string CAMERAD_LONGERROR("longerror"); const std::string CAMERAD_MEX("mex"); const std::string CAMERAD_MEXAMPS("mexamps"); -const std::string CAMERAD_MODE("mode"); const std::string CAMERAD_MODEXPTIME("modexptime"); const std::string CAMERAD_NATIVE("native"); const std::string CAMERAD_OPEN("open"); const int CAMERAD_OPEN_TIMEOUT(10000); @@ -58,6 +58,7 @@ const std::string CAMERAD_USEFRAMES("useframes"); const std::string CAMERAD_WRITEKEYS("writekeys"); const std::vector CAMERAD_SYNTAX = { CAMERAD_ABORT, + CAMERAD_ACFMODE+" [ ? | ]", CAMERAD_AUTODIR, CAMERAD_BASENAME, CAMERAD_BIAS, @@ -88,7 +89,6 @@ const std::vector CAMERAD_SYNTAX = { CAMERAD_LONGERROR, CAMERAD_MEX, CAMERAD_MEXAMPS, - CAMERAD_MODE+" [ ? | ]", CAMERAD_MODEXPTIME, CAMERAD_NATIVE+" ? | [|] [ [ < ARG2> [ [ ] ] ] ]", CAMERAD_OPEN+" [ ? | ]", diff --git a/docs/_ext/camerad_tables.py b/docs/_ext/camerad_tables.py index 50db31a..f6b1a75 100644 --- a/docs/_ext/camerad_tables.py +++ b/docs/_ext/camerad_tables.py @@ -48,13 +48,20 @@ FRAME_OUTPUT_SOURCE = "utils/frame_output_factory.cpp" FRAME_OUTPUT_KEY_PATTERN = re.compile(r'\bkey\s*==\s*"([A-Z][A-Z0-9_]*)"') +# The server hands every name it does not recognize to the controller interface, so the names that +# dispatcher handles are dispatched commands too. Its body runs from the definition to the closing +# banner +CONTROLLER_DISPATCH_SOURCE = "camerad/archon_interface.cpp" +CONTROLLER_DISPATCH_START = "long ArchonInterface::controller_cmd(" +CONTROLLER_DISPATCH_END = "/***** Camera::ArchonInterface::controller_cmd" + # Matched by the dispatch scan but not commands NON_COMMANDS = frozenset({"_EXCEPTION_", "-h", "--help", "help", "?"}) @dataclass(frozen=True) class Command: - """One command the server dispatches, with its advertised syntax.""" + """One command camerad dispatches, with its advertised syntax.""" name: str syntax: str @@ -135,8 +142,7 @@ def _advertised_syntax(header: str, constants: dict[str, str]) -> dict[str, str] return syntax -def _dispatched_commands(server: str, constants: dict[str, str]) -> list[str]: - body = server[server.index("Process commands here"):] +def _matched_commands(body: str, constants: dict[str, str]) -> list[str]: found = [] for constant, literal in re.findall(r'cmd\s*==\s*(?:(CAMERAD_\w+)|"([^"]+)")', body): name = constants[constant] if constant else literal @@ -145,12 +151,23 @@ def _dispatched_commands(server: str, constants: dict[str, str]) -> list[str]: return found +def _dispatched_commands(server: str, controller: str, constants: dict[str, str]) -> list[str]: + """Names the server recognizes, then those the controller interface handles itself.""" + found = _matched_commands(server[server.index("Process commands here"):], constants) + start = controller.index(CONTROLLER_DISPATCH_START) + body = controller[start:controller.index(CONTROLLER_DISPATCH_END, start)] + found += [name for name in _matched_commands(body, constants) if name not in found] + return found + + @cache def load_commands() -> list[Command]: header = _read("common/camerad_commands.h") constants = _command_constants(header) syntax = _advertised_syntax(header, constants) - dispatched = _dispatched_commands(_read("camerad/camera_server.cpp"), constants) + dispatched = _dispatched_commands( + _read("camerad/camera_server.cpp"), _read(CONTROLLER_DISPATCH_SOURCE), constants + ) described = _load_data("commands.yaml") _check_coverage("commands", set(dispatched), set(described)) @@ -329,7 +346,7 @@ def _render(self, headers: list[str], rows: list[list[str]]) -> list[nodes.Node] class CameradCommands(_TableDirective): - """Render the table of commands the server dispatches.""" + """Render the table of commands camerad dispatches.""" option_spec = {"widths": str} diff --git a/docs/architecture/index.md b/docs/architecture/index.md index 91c0918..dca5318 100644 --- a/docs/architecture/index.md +++ b/docs/architecture/index.md @@ -107,15 +107,17 @@ An instrument is a separate repository, checked out as a submodule under 1. A `.cmake` fragment setting `INSTRUMENT_SOURCES`. That is the whole build integration. 2. An `Interface` subclass deriving from `ArchonInterface` or `AstroCamInterface`, overriding what - the detector needs and adding commands through `instrument_cmd()` and `is_instrument_command()`. + the detector needs and adding commands through `instrument_cmd()` and `instrument_commands()`. 3. Its own `ExposureMode` implementations, where the stock ones do not fit. 4. Optionally a FITS header dictionary and shipped `.cfg` and `.acf` files. An interface factory ties it together, so the core builds against the base class and never names a concrete instrument. -Instrument commands are checked *before* the base command chain, so an instrument can add commands -and also override a base one of the same name. +The server recognizes the base commands first, and an instrument changes one by overriding its +method. Any other name goes to `controller_cmd()`, which handles the controller's own commands and +forwards the rest to `instrument_cmd()`. A controller command takes precedence over an instrument +command of the same name. See [instruments](../instruments/index.md) for the modules that exist, and {source}`camerad/Instruments/hispec_tracking_camera` for the most complete example. diff --git a/docs/commands/base.md b/docs/commands/base.md index 9e45507..bb5c878 100644 --- a/docs/commands/base.md +++ b/docs/commands/base.md @@ -1,13 +1,15 @@ # Base commands -Every command the server dispatches. Instrument modules add their own on top of these; see +Every command camerad dispatches. Instrument modules add their own on top of these; see [instruments](../instruments/index.md). Any command accepts `?` as its argument to return its own syntax. -The **Controller** column distinguishes commands the interface implements directly from those -routed through `controller_cmd`, which only the Archon interface implements. On an ARC build the -latter return `not_supported`. +The **Controller** column marks the commands only the Archon interface implements. The server +recognizes the core's commands and calls the interface's function for each. It +hands any other name to `controller_cmd`, where the Archon interface handles its own commands and +forwards the rest to the instrument. On an ARC build the Archon commands return `not_supported` +unless the instrument claims them. ```{eval-rst} .. camerad-commands:: @@ -15,8 +17,8 @@ latter return `not_supported`. ``` :::{note} -This table is generated from the dispatch chain in {source}`camerad/camera_server.cpp`, so it lists -what the server actually answers. The `help` command is a separate hand-maintained list in +This table is generated from the dispatch chains in {source}`camerad/camera_server.cpp` and +{source}`camerad/archon_interface.cpp`, so it lists what camerad actually answers. The `help` command is a separate hand-maintained list in {source}`common/camerad_commands.h` that has drifted: it advertises around two dozen commands the server no longer implements, and omits several it does, including `power`. Trust this table over `help`. @@ -25,5 +27,5 @@ server no longer implements, and omits several it does, including `power`. Trust ## Syntax not shown A few commands are dispatched but absent from the syntax list, so no argument syntax is generated -for them. They are `power`, `getp`, `setp`, `inreg`, `autofetch_mode` and `bob`. Use `?` against a +for them. They are `power`, `getp`, `setp`, `inreg` and `autofetch_mode`. Use `?` against a running server for the authoritative syntax. diff --git a/docs/commands/controller.md b/docs/commands/controller.md index c54f37d..8d9066d 100644 --- a/docs/commands/controller.md +++ b/docs/commands/controller.md @@ -4,8 +4,9 @@ Commands that reach the detector controller rather than being interpreted by the the escape hatch for anything the higher-level commands do not cover. Two routes exist. `native` passes a command straight through and returns the reply. The commands -routed through `controller_cmd`, marked Archon in the [base command table](base.md), are -server-side implementations of controller-specific features. +marked Archon in the [base command table](base.md) are implemented only by the Archon interface. +The server hands any name it does not recognize centrally to +`controller_cmd`, which handles the Archon's own commands and forwards the rest to the instrument. ## Archon @@ -29,7 +30,7 @@ The server itself parses `FRAME`, `STATUS` and `SYSTEM` replies for its own book Beyond `native`, the Archon-specific commands are `raw` for pre-CDS sample capture, `getp` and `setp` for parameters, `inreg` for a VCPU input register, `loadtiming` and `readacf` for loading, -`mode` for camera modes, `autofetch_mode`, and `heater` and `sensor` for the thermal modules. The +`acfmode` for ACF mode sections, `autofetch_mode`, and `heater` and `sensor` for the thermal modules. The [raw samples](#raw-samples) and [heater and sensor](#heater-and-sensor) syntax is below. ## ARC (AstroCam) @@ -38,8 +39,9 @@ Beyond `native`, the Archon-specific commands are `raw` for pre-CDS sample captu `native` is not implemented on an ARC build. `AstroCamInterface::native` logs "not yet implemented" and returns success, so the command replies `DONE` while doing nothing. -`controller_cmd` is not implemented either, so every command in the Archon list above returns -`not_supported` on an ARC build. +`controller_cmd` handles no commands of its own on an ARC build, so the commands in the Archon list +above that the server does not recognize pass to the instrument and return `not_supported` unless +the instrument handles them. ::: Passing three-letter DSP commands through to an ARC controller is therefore not currently possible diff --git a/docs/data/commands.yaml b/docs/data/commands.yaml index 4d17a5f..d64ede6 100644 --- a/docs/data/commands.yaml +++ b/docs/data/commands.yaml @@ -1,17 +1,21 @@ # Descriptions for the commands camerad dispatches. # # The command list and the argument syntax are generated from the source; this file supplies the -# prose. A command added to or removed from the dispatch in camerad/camera_server.cpp without a -# matching change here fails the documentation build. +# prose. A command added to or removed from the dispatch in camerad/camera_server.cpp or +# ArchonInterface::controller_cmd without a matching change here fails the documentation build. # # summary one line, imperative -# controller "any", or "archon" for commands routed through controller_cmd +# controller "any", or "archon" for commands only the Archon interface implements # note optional caveat appended to the summary abort: summary: Abort an exposure in progress controller: any +acfmode: + summary: Apply an ACF mode section directly + controller: archon + autodir: summary: Write images into a dated subdirectory controller: any @@ -32,12 +36,7 @@ bias: bin: summary: Set or get the binning factor controller: any - note: "Broken: the server routes this to the bias handler, so an argument is applied as a bias rather than a binning factor." - -bob: - summary: Reach the BOB interface - controller: archon - note: "Non-functional: no controller implements a handler for it, so it always returns an error." + note: "Not implemented: the Archon interface logs `not yet implemented` and returns an error." close: summary: Disconnect from the controller @@ -87,10 +86,6 @@ loadtiming: summary: Load a timing script and its parameters controller: archon -mode: - summary: Select a camera mode defined in the ACF - controller: archon - native: summary: Send a command straight to the controller and return its reply controller: any diff --git a/docs/python/index.md b/docs/python/index.md index da024c8..cb43886 100644 --- a/docs/python/index.md +++ b/docs/python/index.md @@ -60,11 +60,11 @@ Base commands Instrument commands : `instrument_cmd(command, args)`, a deliberate passthrough rather than one binding each, so a build whose instrument gains a command exposes it with no change to the module. Enumerate with - `instrument_commands()`, or test one with `is_instrument_command()`. + `instrument_commands()`. Controller commands -: `controller_cmd(command, args)` for `mode`, `raw`, `readacf`, `loadtiming`, `heater`, `sensor` and - the other Archon-only commands. +: `controller_cmd(command, args)` for `raw`, `readacf`, `loadtiming`, `heater`, `sensor` and the + other Archon-only commands. It forwards any other name to the instrument, as camerad does. Only `exit` is missing, since the process belongs to the caller. diff --git a/python/camera_interface_module.cpp b/python/camera_interface_module.cpp index 06a7fd9..84da7a2 100644 --- a/python/camera_interface_module.cpp +++ b/python/camera_interface_module.cpp @@ -267,8 +267,8 @@ PYBIND11_MODULE(camera_interface, module) { py::arg("args") = "", py::call_guard(), "Run a named interface test") - // Covers mode, raw, readacf, loadtiming, heater, sensor and autofetch_mode, - // which camerad also routes through controller_cmd + // Runs a command unique to the controller. A name the controller does not + // own is forwarded to the instrument, as camerad does .def("controller_cmd", [](CameraSession &self, const std::string &command, const std::string &args) { std::string retstring; @@ -287,13 +287,6 @@ PYBIND11_MODULE(camera_interface, module) { py::arg("args"), py::call_guard(), "Send a raw command straight to the controller") - .def("is_instrument_command", - [](CameraSession &self, const std::string &command) { - return (*self).is_instrument_command(command); - }, - py::arg("command"), - "Return True if this build's instrument handles the named command") - .def("instrument_commands", [](CameraSession &self) { return (*self).instrument_commands(); }, "Return the instrument-specific command names this build supports") diff --git a/python/examples/raw_channel_sweep.py b/python/examples/raw_channel_sweep.py index 1f5dd25..3ee0178 100644 --- a/python/examples/raw_channel_sweep.py +++ b/python/examples/raw_channel_sweep.py @@ -204,9 +204,11 @@ def run(args: argparse.Namespace) -> int: else: camera.load() camera.power("on") + # Neither load nor readacf selects an exposure mode + camera.exposure_mode("SINGLE") if args.keep_config: - # After power, because selecting a mode applies CDS settings - camera.controller_cmd("mode", args.mode) + # After power, because an ACF mode section applies CDS settings + camera.controller_cmd("acfmode", args.mode) if args.exptime is not None: camera.exptime(str(args.exptime)) @@ -244,7 +246,7 @@ def main() -> int: help="leave the controller's loaded configuration alone, " "reading the ACF host-side only") parser.add_argument("--mode", default="DEFAULT", - help="camera mode to select with --keep-config " + help="ACF mode section to apply with --keep-config " "(default: DEFAULT)") parser.add_argument("--exptime", type=float, help="exposure time, in the unit the config selects") diff --git a/python/tests/emulator_integration.py b/python/tests/emulator_integration.py index 073c699..d5861db 100644 --- a/python/tests/emulator_integration.py +++ b/python/tests/emulator_integration.py @@ -39,14 +39,11 @@ def check_build_identity(expected_instrument: str) -> None: def check_introspection(camera: camera_interface.Camera) -> None: - """Confirm every enumerated instrument command resolves, and no other does.""" + """Confirm the enumerated instrument commands include no unknown name.""" commands = camera.instrument_commands() print(f"instrument commands: {commands}") - for name in commands: - check(camera.is_instrument_command(name), - f"{name} was enumerated but is_instrument_command says no") - check(not camera.is_instrument_command("nosuchcommand"), - "an unknown command was accepted as an instrument command") + check("nosuchcommand" not in commands, + "an unknown command was enumerated as an instrument command") def check_exptime_units(camera: camera_interface.Camera) -> None: @@ -121,6 +118,8 @@ def run(config_path: str, fits_dir: pathlib.Path, expected_instrument: str, camera.open() camera.load() camera.power("on") + # A load selects no exposure mode + camera.exposure_mode("SINGLE") check_exptime_units(camera) diff --git a/reference_instrument/README.md b/reference_instrument/README.md index b6c3a41..e916549 100644 --- a/reference_instrument/README.md +++ b/reference_instrument/README.md @@ -11,9 +11,6 @@ It derives from `ArchonInterface` and overrides the three specialization points, | Instrument configuration | `configure_instrument` | reads `REFERENCE_VALUE` from the cfg, which `refvalue` reports | | Mode selection | `get_exposure_modes`, `set_exposure_mode` | `exposuremode ?` lists `REFERENCE`; `exposuremode REFERENCE` selects it | -`is_instrument_command` is also overridden, only because the current server consults it before every command. That -is not the design, and the dispatch fix removes it. - `REFERENCE` reads out as `SINGLE` and differs only in the type it reports. Any other mode is constructed by `ArchonInterface`. diff --git a/reference_instrument/reference_instrument.cpp b/reference_instrument/reference_instrument.cpp index f5b2d9b..9449ab6 100644 --- a/reference_instrument/reference_instrument.cpp +++ b/reference_instrument/reference_instrument.cpp @@ -8,8 +8,6 @@ #include "reference_exposure_modes.h" #include "utilities.h" -#include - namespace Camera { /***** Camera::ReferenceInstrument::configure_instrument ********************/ @@ -32,23 +30,6 @@ namespace Camera { /***** Camera::ReferenceInstrument::configure_instrument ********************/ - /***** Camera::ReferenceInstrument::is_instrument_command *******************/ - /** - * @brief returns true if cmd is one of instrument_commands() - * @details The server consults this before every core command, so - * refvalue is reached only through it. That gate is not the - * design: a name the core does not recognize is meant to pass - * to the controller and then to the instrument with no gate. - * This override goes when the server does that. - * - */ - bool ReferenceInstrument::is_instrument_command(const std::string &cmd) { - const auto commands = this->instrument_commands(); - return std::find(commands.begin(), commands.end(), cmd) != commands.end(); - } - /***** Camera::ReferenceInstrument::is_instrument_command *******************/ - - /***** Camera::ReferenceInstrument::instrument_commands *********************/ /** * @brief names of the commands this instrument handles @@ -76,8 +57,7 @@ namespace Camera { return this->refvalue(args, retstring); } else { - retstring="unrecognized command"; - return ERROR; + return this->Interface::instrument_cmd(cmd, args, retstring); } } /***** Camera::ReferenceInstrument::instrument_cmd **************************/ diff --git a/reference_instrument/reference_instrument.h b/reference_instrument/reference_instrument.h index 9655a5d..b9e3710 100644 --- a/reference_instrument/reference_instrument.h +++ b/reference_instrument/reference_instrument.h @@ -18,7 +18,6 @@ namespace Camera { public: void configure_instrument() override; - bool is_instrument_command(const std::string &cmd) override; std::vector instrument_commands() const override; long instrument_cmd(const std::string &cmd, const std::string &args,