Zultron/2022 09 19 rebase for PRs: 9 state cmd#21
Closed
zultron wants to merge 100 commits intotormach:foxy-develfrom
Closed
Zultron/2022 09 19 rebase for PRs: 9 state cmd#21zultron wants to merge 100 commits intotormach:foxy-develfrom
zultron wants to merge 100 commits intotormach:foxy-develfrom
Conversation
Fix test_dot test. External projects may have mgr classes without a separate category, i.e. in the `all` class, which causes collisions. I'm not sury why this was ever this way in the first place. Other changes for cleanliness only; nothing fixed: - Remove redundant piece of conditional - Improve assertion exception messages
...and `update()` that interface from `get_feedback()`. This fixes issues with the manager class. It also shows that for a more intuitive interface, interfaces should be reset from a `reset()` method in the base `read()`, and `update()` renamed to `set()`.
Will be used by subclasses
When dumping drive params, uploading some objects is expected to fail; suppressing stderr silences the cryptic & out of context error messages printed by the `ethercat` command
The `dump_param_values()` method uploads all device SDO values and
returns in a dict of `{sdo_obj : value}` pairs.
Remove unneeded fixtures
The dash character in e.g. `-1` confuses the `ethercat` utility. Fix up the `ethercat download` command to explicitly signal the end of options.
The `LCECCommand.upload()` method can now handle string types.
The `munge_config()` method was too rigid, requiring each key to be named. It also clobbered bits of the original. Instead, copy the whole raw device config, avoiding skipped and clobbered keys, and munge just the bits that need munging.
Add objects shown in manual (and seen on drives) but not in original ESI from Inovance - 2002-06h, 2002-07h: Stop mode settings - 2004-18h: Forced DO output in non-OP state - 2005-08h, 2005-0Ah: Electronic gear ratio settings - 200D-03h: Offline autotuning setting
The `index` attribute was unused other than to compute pin names in the `HALDevice` class. Replace it with a generic string used for generated identifiers.
The base `Device` class `index` attribute, unused outside this class, was removed. Generate HAL pins by munging the `address` attribute instead.
LinuxCNC doesn't support these
Add a general `ConfigIO` class. It covers several cases, broken down
into categories:
- Config file source:
- POSIX path for reading or writing
- Python `importlib` resource (possibly in an "egg" zip file)
- Config file use case:
- Open as IOStream object (can use as context manager; has `read()`,
etc. methods)
- Read (path + resource) or write (path only) YAML data
This will clean up config data handling for both the device manager
classes and their tests. It makes sharing config data from package
resources easier, both internally within this package, and also for
use by external packages (the latter being the main motivation for
this change).
Following previous commit for `errors`.
Command and config classes don't have access to device class `name` attribute. This was a design error.
- Generate device config as class method - Update test data for self-consistency
Use `DebugInterface` in tests
Make `init(comp=comp)` arg optional to simplify calling for `HALCompDevice` objects that have already set up `self.comp`. Move `comp.ready()` directly into `HALCompDevice.init()` for cleanliness.
For interface keys with type like e.g. `str` with no equivalent HAL type, just omit them during HAL comp pin creation. This will be used in the `mgr_hal` module, which creates interface keys like `state_log`, a human-readable string not meant for a HAL pin.
Create pins from interfaces, not from pin specs. This is a step toward creating pins for dynamic interfaces whose keys are not necessarily defined in the static interface definitions. The manager needs this for automatic device discovery. Make `init(comp=comp)` arg optional to simplify calling for `HALCompDevice` objects that have already set up `self.comp`. Move `comp.ready()` directly into `HALCompDevice.init()` for cleanliness.
Remove HAL IO pin capability, which hasn't worked out well in practice. This simplifies a lot of logic in both `init()` and `read()`/`write()`.
In order to support homing devices individually, the manager must expose separate `home_request` command for each drive. This is a substantial change, since some device command still comes from the manager, and now some comes from the outside. The manager's `command_in` interface is no longer static, and per-device keys must be added after drives are enumerated during the bus scan. The incoming command is also changed so that command doesn't anymore come in through the `feedback_in` interface and pass through `feedback_out`, and pass into `command_in` after manager munging. As a result, the feedback interfaces also change quite a bit. Because of the dynamic changes to initialization, the many init functions are combined so that using code can simply create the manager object and call the single `init()` method. No more separate `init()`, `init_sim`, `init_devices()`. Many (though not yet enough) CiA 301 & 402 functions were moved out of the manager and into the devices so the manager can be device agnostic and manage more types of devices. This is true of homing (completely removed from the state machine), drive control mode, and status & control word flags. This simplifies the top-level manager logic. It also requires big changes to the tests, whose routines are adapted for this new scheme, and test cases are extensively updated.
Another artifact of the previous screwey design where the manager command was passed through the `feedback_in` interface, the keys `state_log`, `command_complete`, `reset` and `drive_state` were part of the `command_in` interface. This commit moves them to the `command_out` interface where they logically belong. In a similar way, this commit also fixes the handling of the `state_cmd` key: The previous screwey design overrode that key in the `command_in` interface, when it should have been overridden only in the `command_out` interface. This doesn't make sense, and also breaks tests when `command_in` is set from test data, but then changes before the interface is checked (again) against test data.
There's a chicken and egg problem in `init()`, where the parent `init()` initializes interfaces, but `init_devices()` hasn't yet created device-related interface keys. This may not yet be fully resolved, but this is a crude attempt that moves device interface key creation to `init_devices()`. It may still be a problem in the HAL manager, since the HAL comp device `init()` method creates the HAL comp, creates interfaces with HAL pins, and then marks the comp ready.
Updates for `override_interface_param()` change
The `HALHWDeviceMgr` class inherits from `HALCompDevice` before `HWDeviceMgr` so that the former's `init()` function initializes the HAL comp before the latter's `init()` function runs, and afterwards, marks the HAL comp `ready()`. Set empty `slug_separator` to avoid prefixing pins with e.g. `.reset`.
Update for `init()` rework: - Update method signature - Fold reading device config and sim device data into main `init()` method - Update tests Also, `update_rate` was moved from class attr to `mgr_config` key.
Update entrypoint with simplified `init() call.
Decouple state command and feedback and convert command to integer type so that state command can be tied to a HAL pin. To support this, add e.g. `STATE_INIT` & other attributes to define each state's int type.
When the mgr follows state_cmd, but can't write it, faults are automatically cleared, and everything gets messed up. Instead, add a separate signal to command mgr to switch state on rising edge.
…-19-rebase_for_PRs-3-minor-core
Add support for Beckhoff CU1128 Junction Box
…ltron/2022-09-19-rebase_for_PRs-4-config_io
…tron/2022-09-19-rebase_for_PRs-5-pdo_config
…ltron/2022-09-19-rebase_for_PRs-6-errors
…n/2022-09-19-rebase_for_PRs-7-fault_handling+sdo_abstraction
…abstraction' into zultron/2022-09-19-rebase_for_PRs-8-refactor_init+hal_pin_handling
…in_handling' into zultron/2022-09-19-rebase_for_PRs-9-state_cmd
Collaborator
Author
|
Superseded by #22. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is 9 of 9 in a series of PRs refactoring the
hw_device_mgrto run outside ROS, handle file resources more intelligently, manage PDO configurations, improve abstraction, improve CiA402 homing, simplify initialization, fix astate_cmdrace condition, and other fixes and improvements.This ninth and final PR is based on & should be merged after #20, and cleans up state command handling, resolving a race condition.
Previously, the shared but single
state_cmdattribute was read fromcommand_in(to oversimplify a gross hack), and the manager would decide whether to override commanded state (e.g. given a drive fault) before writing tocommand_out. The race condition occurred when a new command arrived afterstate_cmdwas read but before it was written: in between, incoming command was ignored, and at the end, any change was overwritten.The now non-shared
state_cmdattribute, representing state desired by higher controls or user, is now directly read fromcommand_in. The actual state is written simply asstateto thecommand_outinterface. The race condition is no longer an issue, since incoming command will not be clobbered, and even if it arrives in the middle of the update function, the change will still be picked up on the next cycle.