ArcEye
commented
6 years ago 
ok, plans revised. On the spectrum "proper class encapsulation" to "best HAL integration" I'd go for the latter and handle all these variables as potential HAL pins (they easily can, but they do not have to be pins). It would mean tp becomes HAL-aware, which it currently is not. It would mean value access is by direct data reference, not getters/setters. There can still be "change callbacks" out of tp, but they might be optional (NULL allowed).
this means: all shared variables and parameters become hal_s32_t * and hal_float_t * pointers in a struct which will be allocated via hal_malloc() at tpInit() time. This should also be compatible with future configItems once we have them (it might mean configItem values need to live in the HAL shm segment).
The emc/tp code needs access to some config and status items. This is currently done by referencing the emcmot_config_t, emcmot_status_t and emcmot_joint_t structures directly. This is undesirable as it creates a data structure dependency, and makes decoupling harder.
The idea is to replace those references by callbacks which go through a struct passed down from using code; the tp would get/set values as needed through these callbacks. If a callback is NULL for a readonly value, a default value could be used instead.
These are the items referenced:
num_dio, num_aio - setup time, read-only - pass as parameters once
emcmot_config_t (all read-only): maxFeedScale double arcBlendGapCycles double arcBlendOptDepth int arcBlendEnable int arcBlendRampFreq double arcBlendFallbackEnable int
Plan: make those get_* callbacks to fetch values as needed Q1: do we need for changing these values on the fly? (like making them a pin/config item?) Q2: if callbacks not set, what would be sensible default values?
emcmot_status_t
read-only - plan: make those getter callbacks: net_feed_scale spindleRevs spindle.direction enables_new spindle_is_atspeed spindleSpeedIn Q: any sensible default values for the above?
read-write - make those getter/setter callback pairs; I guess these are must-have callbacks: spindleSync current_vel dtg spindle_index_enable spindleSync
write-only - make those setters, ignoring them if NULL callbacks requested_vel distance_to_go spindle.speed enables_queued
velocity and acceleration bounds from emcmot_joint_t: these look like config-time fixed (or does it make sense have those changeable on the fly?) tpGetMachineAccelBounds(PmCartesian * const acc_bound) tpGetMachineVelBounds(PmCartesian * const vel_bound)
the alternative to a table of getters/setters would be to provide pointers to the raw data (wherever that lives) decoupled from the actual structures; while simpler, we dont get change notification in the using code - the callback scheme would allow tacking an action onto any change
another method would be to move these items into the tp instance data, and provide getters/setters for the using code
what is preferrable: callbacks, data reference or make it instance data?
it's also conceivable to use a combination; e.g an using-code accessor for say current_vel, requested_vel etc; and callbacks for spindle* values which change rarely.