Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zultron/2022 09 19 rebase for PRs: 8 refactor init+hal pin handling #20

Draft
wants to merge 94 commits into
base: foxy-devel
Choose a base branch
from

Conversation

zultron
Copy link
Collaborator

@zultron zultron commented Sep 20, 2022

This is 8 of 9 (7 skipped) in a series of PRs refactoring the hw_device_mgr to run outside ROS, handle file resources more intelligently, manage PDO configurations, improve abstraction, improve CiA402 homing, simplify initialization, fix a state_cmd race condition, and other fixes and improvements.

This eighth PR is based on & should be merged after #19, and does the following.

Fault handling, homing: (originally meant to be the seventh PR)

  • Plumb fault through the feedback_out interface in the cia_301 subclesses, used by cia_402 subclasses
  • Implement generic parameter initialization in the cia_301 config classes
  • Clean up SDO loading in tests

Refactor init() and HAL pin handling:

  • Device class interfaces are made dynamic so the manager supports adding per-device attributes; used to expose per-drive homing command and feedback for configurations with variable numbers of drives
  • Implement homing in cia_402 and plumb controls through to top-level manager
  • Rework multitude of init methods to reduce complexity of scripts
  • Clean up HAL pin configuration: create pins from interfaces, following dynamic interface changes
  • Rework top-level manager controls:
    • Cmd and feedback actually goes through feedback_out and command_in interfaces
    • The wacky & hacky scheme of side-loading through feedback_in and command_out and its limitations are gone
    • Attributes now can't be shared between interfaces, so no more HAL I/O pins; this clears the way to fix the state_cmd race condition

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()`.
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
- Update for test case override data
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.
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant