LinuxCNC / linuxcnc

LinuxCNC controls CNC machines. It can drive milling machines, lathes, 3d printers, laser cutters, plasma cutters, robot arms, hexapods, and more.
http://linuxcnc.org/
GNU General Public License v2.0
1.81k stars 1.16k forks source link

get rid of [EMCMOT]TRAJ_PERIOD #306

Open SebKuzminsky opened 7 years ago

SebKuzminsky commented 7 years ago

@WJHildreth reports in #249 that several ini variables are either undocumented or unused.

Casual inspection shows that [EMCMOT]TRAJ_PERIOD is documented but not used. The trajectory planner runs as part of Motion, in the servo thread, not in its own thread, so it makes no sense for it to have a period of its own.

andypugh commented 7 years ago

On 14 July 2017 at 22:17, Sebastian Kuzminsky notifications@github.com wrote:

@WJHildreth https://github.com/wjhildreth reports in #249 https://github.com/LinuxCNC/linuxcnc/issues/249 that several ini variables are either undocumented or unused.

Casual inspection shows that [EMCMOT]TRAJ_PERIOD is documented but not used. The trajectory planner runs as part of Motion, in the servo thread, not in its

So that's both [TRAJ]CYCLE_TIME and [TRAJ]TRAJ_PERIOD that should go?

-- atp

SebKuzminsky commented 7 years ago

Yes, both ini variables should go, [TRAJ]CYCLE_TIME and [TRAJ]TRAJ_PERIOD, in master but not in 2.7 (in case someone's using the ini variables in custom hal files or anything like that).

Motion has a command-line argument/modparam named traj_period_nsec, that should go (we don't create a traj thread).

There's some stuff inside linuxcnc to track the servo thread period, it gets placed into the Status struct as traj.cycle_time, that should stay, thought maybe a rename or a move is in order.

There's some related stuff inside Motion that'll need to be cleaned up, emcmotConfig's interpolationRate, the thread cycle time helper functions, the cubic kinematics of the Free/Joint-mode planner.

There may be other stuff to clean up, this is just from a quick look around. But the ini stuff can be cleaned up independently of cleaning the inside of Motion.

nicokid commented 7 years ago

Hi Seb,

Then I have to remove from my stepconf the following variables: [EMCMOT]COMM_WAIT [EMCMOT]TRAJ_PERIOD [TRAJ]CYCLE_TIME

It's correct? Nicola.

SebKuzminsky commented 7 years ago

[TRAJ]CYCLE_TIME and [EMCMOT]COMM_WAIT should definitely go. That part is correct.

[EMCMOT]TRAJ_PERIOD is a little more complicated, it's used in a surprising and non-standard way inside the Motion controller, and @jepler pointed out on IRC that we should understand that usage better before removing it.

SebKuzminsky commented 7 years ago

Wait, after re-reading your comment I'm not sure I understand what you're asking.

I already modified the stepconf program so that it no longer writes [EMCMOT]COMM_WAIT or [TRAJ]CYCLE_TIME in the .ini files it creates (a5c7736af80bb4e357b4db4ec126c6347f5e999f and 945de033f9bee05c311dd95a64793c2efcd43b79).

Are you asking about an existing .ini file that you have, that was created by a version of stepconf that's older than Friday July 14, 2017? ini files like that will still have those two variables, and they can safely be removed by hand.

nicokid commented 7 years ago

I have a ton of changes to Stepconf. Some are in the #274 pull request, others are bugfixes for integrating gladevcp (I have not posted yet).

Nicola.

andypugh commented 7 years ago

I just pushed a patch to expunge COMM_WAIT and CYCLE_TIME in the update_ini script

-- atp