search

nasa-jpl / fastcat

C++ EtherCAT Device Command & Control Library
Other
47 stars 11 forks source link

Rename actuator classes #87

Closed alex-brinkman closed 1 year ago

alex-brinkman commented 1 year ago

Renames:

I've added Actuator class parsing support to backwards compatibility until we release v1.0.0 eventually.

[ WARN  ] [1680539897.571716] (manager.cc:644) Starting in v0.12.0, Platinum device support has been added to Fastcat!                                                                               
[ WARN  ] [1680539897.571719] (manager.cc:645) Therefore the 'Actuator' class has been renamed to the 'GoldActuator' to make room for the new 'PlatinumActuator' device                              
[ WARN  ] [1680539897.571721] (manager.cc:646) Support for 'Actuator' is eventually going to be dropped in v1.0.0 so update your YAML now to prevent testbed downtime
alex-brinkman commented 1 year ago

With this merge complete, I'll mint v0.12.0 and unveil the platinum driver to the world!

d-loret commented 1 year ago

@alex-brinkman, we might still want to make it a major release since, even not modifying Actuator, it will break backward compatibility with client applications simply because of the splitting of state.

alex-brinkman commented 1 year ago

So prior to v1.0.0, I'm falling back to this rule: Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable. https://semver.org/#spec-item-4 For V0.y.z development, we treat Major releases as Minor releases and both Minor/Patch releases as a Patch release. i.e. everything shifts over on number.

I've found that as soon as we mint v1.0.0 after a big development thrust, we tend to need to jump to v2.0.0 or even v3.0.0 really quickly, so I'd prefer to let the dust settle on these change for a few months and target the v1.0.0 release for this summer, aligning with other application schedules.

d-loret commented 1 year ago

So prior to v1.0.0, I'm falling back to this rule Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

Hehe, we should be in this rule indefinitely.

My comment stem from here.

alex-brinkman commented 1 year ago

I'd rather not have the Actuator Class be backwards compatible so I'm going to go ahead and remove the special handling of this string.

alex-brinkman commented 1 year ago

If a topology contains Actuator it now is rejected but with helpful deprecated info still:

[ WARN  ] [1680729778.168764] (fastcat/src/manager.cc:657) Starting in v0.12.0, Platinum device support has been added to Fastcat!
[ WARN  ] [1680729778.168767] (fastcat/src/manager.cc:658) Therefore the 'Actuator' class has been renamed to the 'GoldActuator' to make room for the new 'PlatinumActuator' device
[ ERROR ] [1680729778.168770] (fastcat/src/manager.cc:659) Update your topology for all 'Actuator' entries
[ ERROR ] [1680729778.168773] (fastcat/src/manager.cc:140) Failed to configure Offline bus
[SUCCESS] [1680729778.168776] (fastcat/test/test_config.cc:23) File successfully parsed!
[SUCCESS] [1680729778.168779] (fastcat/src/manager.cc:78) Freed JSD memory
alex-brinkman commented 1 year ago

As for the naming, I don't remember what was the consensus for the name of the base class, wasn't it ActuatorBase or something along those lines? I don't have a preference on this one, but I was left wondering.

The internet does not show consensus here so let's leave it to reduce code change.