Skip to content

Conversation

@c-morley
Copy link
Collaborator

peck distance for g73/g83
peck count for g73
g73 does short pecks to break chips.
If count is >0, it will fully retract to clear chips every 'count'
number of pecks

[RS274NGC]
PARAMETER_DRILL_CYCLE_CHIP_BREAK_DISTANCE = .020
PARAMETER_G73_PECK_TILL_CLEAR_COUNT = 2

@c-morley
Copy link
Collaborator Author

Hmm I guess setting these in machine units would make more sense.
I'm now not sure if peck and retract is a good drilling strategy.

CHKS((delta <= 0.0), NCE_NEGATIVE_OR_ZERO_Q_VALUE_USED);

rapid_delta = G83_RAPID_DELTA;
rapid_delta = _setup.parameter_drill_cycle_chip_break_distance;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not change the definition of G83_RAPID_DELTA instead? If G83_RAPID_DELTA is no longer used, it should be removed from src/emc/rs274ngc/interp_internal.hh. I suspect it is better to keep using it, but changing its definition to _setup.parameter_drill_cycle_chip_break_distance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly because the G83_RAPID_DELTA variable is a constant.
I think it is also more informative to see where the value came from. (_setop)

yes G83_RAPID_DELTA should be removed.

@c-morley c-morley force-pushed the cmorley/add_drill_cycle_paremeters branch from 4996c95 to ab23f04 Compare July 30, 2022 02:17
@c-morley
Copy link
Collaborator Author

Hmm I think I'm still missing a fix for g20/g21 mode changes at run time.

@cakeslob
Copy link

"peck distance for g73/g83"
"PARAMETER_DRILL_CYCLE_CHIP_BREAK_DISTANCE = .020"

Hey, what exactly does this mean?
Is this a retract distance for G73 chip breaking?
How does this change G83? Is this the distance it will rapid to before it starts to feed in the peck cycle?

@c-morley
Copy link
Collaborator Author

.02 inch or mm equivalent

In g83 it rapids back .02 less then last feed position

@petterreinholdtsen
Copy link
Collaborator

One comment in <URL: https://yewtu.be/watch?v=C253C9ODMn4 > made me suspect its user too would like to be able to tune the pecking.

@SebKuzminsky
Copy link
Collaborator

This looks like a good idea.

Needs documentation.

I don't think the prefix PARAMETER_ on the INI settings and parameter_ on the internal variable names adds anything, other than finger strain.

I think there's a unit type bug in this code. The value of _setup.parameter_g83_peck_clearence (and 73) can be in metric or imperial, depending on the machine config (as determined by _setup.length_units). But the units of the G-code program are totally independent of that. I think the usual way this is handled is for LinuxCNC to store its distances in the internal representation (metric) and perform unit conversion as needed at the point of use (but it's been a while since I was in this code, please fact check me on this).

@c-morley
Copy link
Collaborator Author

I'll have to check the code too - been a long time.

The parameter name is to give a clue where the data came from - I believe Fanuc uses this terminology.
My thought is there will be more in the future.

As programmed, the parameter setting should be in machine units in the INI and is converted at runtime to what ever the current system units are. This seems more straight forward then converting it twice (for an imperial machine in imperial sytstem units) but maybe this is inconstant with other things in linuxcnc?

From memory I believe originally it was hard coded in inches and not converted at all.

@SebKuzminsky
Copy link
Collaborator

The Hackfest Review Committee deems:

  • Needs docs.
  • Misspeling of variable in the patch, should be "clearance" not "clearence".

c-morley added 5 commits May 20, 2023 14:33
peck distance for g73/g83
peck count for g73
g73 does short pecks to break chips.
If count is >0, it will fully retract to clear chips every 'count'
 number of pecks

[RS274NGC]
PARAMETER_DRILL_CYCLE_CHIP_BREAK_DISTANCE = .020
PARAMETER_G73_PECK_TILL_CLEAR_COUNT = 2
@c-morley c-morley force-pushed the cmorley/add_drill_cycle_paremeters branch from ab23f04 to 9aefbe5 Compare May 20, 2023 22:10
@rene-dev
Copy link
Member

Is there any particular reason for an ini parameter but not a parameter to the gcode?

@c-morley
Copy link
Collaborator Author

c-morley commented May 20, 2023

I believe it's because it's not something you would tend to change often but this does allow you to change it to a preference.

@rene-dev
Copy link
Member

The Norwegian linuxcnc meeting decided this should be a gcode letter, not a ini parameter. its tool specific, not a machine specific. Changing it from ini to gcode later is probably a bad idea.

@c-morley
Copy link
Collaborator Author

Well I'm not sure who the Norwegian linuxcnc meeting is, but i would think it's usual to have the discussion to include the author.

@rene-dev
Copy link
Member

I don't know what makes you feel excluded from the discussion. We (@petterreinholdtsen @andypugh @hansu ) went thru all the PRs, like we did at the Madison meeting, and just wrote down our thoughts. I opened a new PR which includes an optional P word.

@c-morley
Copy link
Collaborator Author

Because a decision was made at a venue I'm not at nor AFAIK can participate in by an umknown group when in fact I had no idea that it was being 'decided on'.
No extra discussion was added here or in the maillist - the places that are designed for this.

In this particular case I don't care much about this PR - it was a request from someone else that I thought would be good practice.

But process matters - if you want want people to participate with PRs and a a healthy project that attracts new people, you must stick with the process that is the norm or otherwise try to communicate in a way that all interested people can be involved.

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.

6 participants