-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Gmoccapy - remove AUTOMATIC_G43 #3113
base: master
Are you sure you want to change the base?
Conversation
Gmoccapy had automatic activation of tool offsets using the G43 command after a tool change. The automatic G43 caused "race condition problems" in some configurations. That's why it was removed. Automatic G43 should be implemented by LCNC remap functionality not GUI.
Added python code for M61 in /configs/sim/gmoccapy/python/stdglue.py
My fault - i cannot remove commit from Github
Since the bug with M61 disabling MDI mode is not related to this PR I would suggest to remove this particular issue from the description of this pull request and file it in the 'Issues' list. |
as for the broken configs, try these modifications: in 'configs/sim/gmoccapy/lathe_configs/lathe_macros.ini' replace this line: in configs/sim/gmoccapy/non_trivial_kinematics/table-rotary-tilting/xyzac-trt.ini |
Could you edit the initial comment with the bug description and remove everything regarding the M61 bug and move that over to the issue you opened about this #3120 This way it would be much clearer that the two are not related. |
After solving the bugs I will mark them [SOLVED]. I'll test your solution suggestions tonight. I hope it's clearer now. |
My apologies, I was under the impression that the m61 bug was not related to your changes to the ini file. [edit] |
Testing 'configs/sim/gmoccapy/lathe_configs/lathe_macros.ini' with my suggested I actually get errors:
the issue here is that the original So this is really something that was already broken in all the configs inside a sub folder of 'configs/sim/gmoccapy'. In none of those configs can we use for example 'o<i_am_lost>call'. Fixing this seems to be somewhat tricky because of this path that is used inside the macro ngc files:
So I would suggest to move your two new ngc files into a new 'subroutines' folder and then to add that to the 'SUBROUTINE_PATH' as either ':subroutines' or ':../subroutines' depending on whether the .ini file is located in the 'configs/sim/gmoccapy' folder or a subfolder. |
repaired SUBROUTINE_PATH = ./examples:../../macros
I have a question for @hansu . I wanted to add a note about deleting AUTOMATIC_G43 here: Currently the version of Gmoccapa is 3.4.8. Unfortunately, this designation is the same in both stable (2.9) and master (2.10). Will there be a change for Gmoccapa versioning? In the past, most PRs was merged into STABLE, so versioning was not a problem. But now there is pressure to have only BUGfix in STABLE. (I agree with this, but grey zone exist) So maybe it would also require a change in versioning, or its definition. This PR is again an exception. This is a BUGfix, but it changes the behavior a lot, so it must belong to the MASTER branch. |
We can increase the version number of gmoccapy for LinuxCNC 2.10 to 3.5.0. |
Can I ask you explain why? I am interesting. |
Because the glade file is generated by the Glade Editor and sometimes it happens that the order changes, maybe by a version change or something different. and if you then change something within those lines ... good luck :) |
@@ -1,3 +1,9 @@ | |||
ver 3.5.0 | |||
- version for 2.10 (master) branche |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- version for 2.10 (master) branche | |
- version for 2.10 (master) branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
@@ -54,7 +54,15 @@ py = python3 | |||
[RS274NGC] | |||
RS274NGC_STARTUP_CODE = G17 G21 G40 G43H0 G54 G64P0.005 G80 G90 G94 G97 M5 M9 | |||
PARAMETER_FILE = sim.var | |||
SUBROUTINE_PATH = macros | |||
SUBROUTINE_PATH = ./macros |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some INI files is ../macros or ../../macros needed. I wanted to unify it.
I think, that exist 2 ways:
Did you think about second way? |
That would prevent you from adding new graphical features to Gmoccapy, so no real option.
That's only the "emergency option" when merging is not really feasible. We only have to keep an eye of the changes in the glade file before merging and try to fix the 2.9 glade file manually or make sure Glade does not change the order of elements. |
I am confused now. HansU:
Then: zz912:
HansU:
What is the procedure for bugfix to 2.9 gmoccapy.glade? |
Just keep the changes in the glade file in 2.9 small. |
Try making just the glade changes in (local) 2.9, then see if you can merge them into a local version of master. |
Gmoccapy had automatic activation of tool offsets using the G43 command after a tool change. The automatic G43 caused "race condition problems" in some configurations. That's why it was removed. Automatic G43 should be implemented by LCNC remap functionality not GUI.
This PR fix many bugs:
#2489
#2613
#2453
......
Here rmu75 explain why automatic_G43 is not good in GUI:
#2841 (comment)
Here Norbert allow me remove automatic_G43 from Gmoccapy:
#2841 (comment)
Sorry that this PR contains a lot of commits. I can only use Github in a web browser.
Unfortunately, this patch produced, or rather pointed out, more bugs:
using M61 disable MDI window [SOLVED - this bug is not related with this PR]
remap broke these configurations: lathe_macros.ini, xyzac-trt.ini [SOLVED]
I would like ask for help.