Skip to content

Conversation

@zz912
Copy link
Contributor

@zz912 zz912 commented Jun 15, 2025

Reported here: https://forum.linuxcnc.org/gmoccapy/56079-change-tool-offsets-after-editing-the-tool-table?start=0

Toolinfo is updated after click on Apply button in tooltable page.
Update_toolinfo_after_Apply

@Sigma1912
Copy link
Contributor

Looking at the changes makes me think that this does actually two things after clicking on the 'Apply' button.

  1. Update the tool information
  2. Issue a mdi 'G43' command if 'G43' is already active

I think it would be nice if your commit message

  • would reflect the changes the commit introduces
  • use the english language

Remember that these commit messages are stored in the git tree and provide information to future developers.

@zz912
Copy link
Contributor Author

zz912 commented Jun 16, 2025

Looking at the changes makes me think that this does actually two things after clicking on the 'Apply' button.

  1. Update the tool information

Update only tool diameter and toolname not offset Z (+X for lathe)
I copied this part of the code from here:

toolinfo = self.widgets.tooledit1.get_toolinfo(tool)
if toolinfo:
# Doku
# toolinfo[0] = cell toggle
# toolinfo[1] = tool number
# toolinfo[2] = pocket number
# toolinfo[3] = X offset
# toolinfo[4] = Y offset
# toolinfo[5] = Z offset
# toolinfo[6] = A offset
# toolinfo[7] = B offset
# toolinfo[8] = C offset
# toolinfo[9] = U offset
# toolinfo[10] = V offset
# toolinfo[11] = W offset
# toolinfo[12] = tool diameter
# toolinfo[13] = frontangle
# toolinfo[14] = backangle
# toolinfo[15] = tool orientation
# toolinfo[16] = tool info
self.widgets.lbl_tool_no.set_text(str(toolinfo[1]))
self.widgets.lbl_tool_dia.set_text(toolinfo[12])
self.halcomp["tool-diameter"] = float(locale.atof(toolinfo[12]))
self.widgets.lbl_tool_name.set_text(toolinfo[16])

  1. Issue a mdi 'G43' command if 'G43' is already active

G43 update offset Z. If G43 is not active, I don't activate it unnecessarily.

What are you asking, or what should I do?


I think it would be nice if your commit message

would reflect the changes the commit introduces
use the english language

Remember that these commit messages are stored in the git tree and provide information to future developers.

"Prvni pokus" means "First test". It is a bug.
I spent a lot of time trying to fix it, but it didn't work.

Theoretically, "git commit --amend" should work.
https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/changing-a-commit-message
But it didn't work for me, or I guess I just didn't figure out how to use it.

Is there any way to fix it now?

@Sigma1912
Copy link
Contributor

Have you tried to force push the amended commit to your github repo?

git push --force-with-lease origin upd-gui-aft-apply

@zz912
Copy link
Contributor Author

zz912 commented Jun 16, 2025

Have you tried to force push the amended commit to your github repo?

git push --force-with-lease origin upd-gui-aft-apply

Thank you for help, but I dont understand main phylosophy of git. I think, that git is program without "Undo", every change can be made by another commit, but Pull Request with several commits are problem for developers LCNC.

I can make Pull Request, but another changes are problem for me.

@Sigma1912
Copy link
Contributor

Pull Request with several commits are problem for developers LCNC.

If I remember correctly you once had a pull request with a lot of commits just fixing errors in earlier commits. That is a problem.
Commits are meant to organize changes. There is no definite right and wrong and I am certainly no authority but maybe this helps:
https://linuxcnc.org/docs/html/code/contributing-to-linuxcnc.html#_effective_use_of_git

@Sigma1912
Copy link
Contributor

You can also 'squash' several commits into a single one:
https://www.datacamp.com/tutorial/git-squash-commits

@andypugh andypugh requested a review from gmoccapy June 16, 2025 23:53
@zz912 zz912 force-pushed the upd-gui-aft-apply branch from aaf05e7 to be147da Compare June 17, 2025 17:52
@zz912
Copy link
Contributor Author

zz912 commented Jun 17, 2025

Hello Sigma1912,

thank you for:

  • help with change commit message
  • squash link

What should I do if I have a bug in my Pull Request?

  • thanks to your link to squeeze, I now know that I can make a fix commit and then squeeze it.
  • is there any other option?

@hansu
Copy link
Member

hansu commented Jun 17, 2025

What should I do if I have a bug in my Pull Request?

If you have only one commit than the easiest way is to amend (git commit --amend ) the change and force-push that to your branch.

If you have multiple commits, you can do an interactive rebase and change the regarding commit.

Sure, you can also add a commit on top that fixes it, but I wouldn't recommend that in a PR.

@zz912 zz912 force-pushed the upd-gui-aft-apply branch from be147da to 4552c79 Compare June 18, 2025 06:21
@zz912
Copy link
Contributor Author

zz912 commented Jun 18, 2025

I have reworked the comments in gmoccapy.py.
I have changed:
# Automatic G43 => # Reactivate G43 to update offset Z and offset Y

git commit --amend worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants