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.75k stars 1.14k forks source link

pncconf: generate connections to display tooloffset in gmoccapy #2838

Closed hansu closed 4 months ago

hansu commented 6 months ago

See https://github.com/LinuxCNC/linuxcnc/issues/2820 Not tested.

hansu commented 6 months ago

Maybe we should distinguish whether it's a lathe or not and don#t add x in this case

zz912 commented 6 months ago

Maybe we should distinguish whether it's a lathe or not and don#t add x in this case

If you do that, it will be nicer, there won't be unnecessary lines in the configuration, but I don't consider it necessary. Gmoccapy will also create both HAL pins for the milling machine. If you want to solve it, I would also change it to gmoccapy so that it does not create an unnecessary HAL pin for the milling machine.

I would add a blank line here to make it look better: add_line

I tested it and your part code works.

But I found that the gmoccapy_postgui.hal file is not overwritten after modifying the configuration by PNCconf.

So those who will make a new configuration will have new lines added in the gmoccapy_postgui.hal file. Unfortunately, those who will only edit their configuration will not have new lines.

I suggest remaking the gmoccapy_postgui.hal file to a file that is always overwritten. If you decide to do so, it will be necessary to add to the file:

# If you make changes to this file, they will be
# overwritten when you run PNCconf again

When this file is converted to rewriteable, it would add this:

net spindle-vel-cmd-rpm-abs        => gmoccapy.spindle_feedback_bar
net spindle-at-speed                     => gmoccapy.spindle_at_speed_led
net unlock-settings                        => gmoccapy.unlock-settings

I tested this HAL lines and it works.

I also thought about these pins: gmoccapy.warning-confirm IN gmoccapy.blockdelete IN gmoccapy.delete-message IN gmoccapy.error OUT gmoccapy.warning-confirm IN h-button IN v-button IN

hansu commented 6 months ago

Maybe we should distinguish whether it's a lathe or not and don#t add x in this case

If you do that, it will be nicer, there won't be unnecessary lines in the configuration, but I don't consider it necessary. >Gmoccapy will also create both HAL pins for the milling machine.

The sample configs have both, so I will keep both to keep it simple.

I would add a blank line here to make it look better:

done

I suggest remaking the gmoccapy_postgui.hal file to a file that is always overwritten.

@c-morley what do you think? I would tend to keep the current behaviour.

zz912 commented 6 months ago

I suggest remaking the gmoccapy_postgui.hal file to a file that is always overwritten.

@c-morley what do you think? I would tend to keep the current behaviour.

I think, for changing stuffs is custom_postgui.hal.

If you keep the current behavior, please add: # This file will not be overwritten when you run PNCconf again

c-morley commented 6 months ago

On qtvcp, we can add multiple post gui files and aksi post gui commands. If we added this to gmoccapy and Axis, then pncconf can overwrite the file as a user can add a different named oostgui. I think I have the mods done for gmoccapy in my qt nativecam work.

Anyways I think that's what I would do.

c-morley commented 6 months ago

Gmoccapy postgui/command patch: https://github.com/LinuxCNC/linuxcnc/commit/3d86d37c5c15f4f0ba1e0346e035fd8d8ebe62bb

hansu commented 6 months ago

I would define two files as "POSTGUI_HALFILE" One custom_postgui.hal and one gmoccapy_postgui.hal or similar. And the latter will be generated by pncconf.

zz912 commented 6 months ago

I would define two files as "POSTGUI_HALFILE" One custom_postgui.hal and one gmoccapy_postgui.hal or similar. And the latter will be generated by pncconf.

I don't understand you, what you are writing about is the default behavior of PNCconf. Here is the default generated configuration: postgui-001 postgui-002

hansu commented 6 months ago

Sorry I never used pncconf. Then it's save to overwrite the gmoccapy_postgui.hal I think.

c-morley commented 4 months ago

I would define two files as "POSTGUI_HALFILE" One custom_postgui.hal and one gmoccapy_postgui.hal or similar. And the latter will be generated by pncconf.

I don't understand you, what you are writing about is the default behavior of PNCconf. Here is the default generated configuration: postgui-001 postgui-002

Yes i seems this in in master already.

hansu commented 4 months ago

I would define two files as "POSTGUI_HALFILE" One custom_postgui.hal and one gmoccapy_postgui.hal or similar. And the latter will be generated by pncconf.

I don't understand you, what you are writing about is the default behavior of PNCconf. Here is the default generated configuration: postgui-001 postgui-002

Yes i seems this in in master already.

But not in 2.9? Then it's probably a bad idea to back port it to 2.9?

@zz912 What's the status of this PR including the new facts? What need to be done? I am a bit out of focus, sorry.

zz912 commented 4 months ago

I am confused by c-morley too with his sentence aboat master. My picture above are from 2.9 branche. I dont use master branche.

Edit: I think I understand Chris. He wants this Pull Request to be in branch master not in 2.9. We should respect the wishes of Chris, as the author of PncConf.

zz912 commented 4 months ago

Summary of my wish for this PR: 1) Commit into master branch. 2) Make the gmoccapy_postgui.hal file rewritable by PncConf 3) Draw attention to this at the beginning of the file with the text:

# If you make changes to this file, they will be
# overwritten when you run PNCconf again

This is a standard warning of PncConf not my invention.

4) We should agree whether we will also add this:

net spindle-vel-cmd-rpm-abs        => gmoccapy.spindle_feedback_bar
net spindle-at-speed                     => gmoccapy.spindle_at_speed_led

I tested this HAL lines and it works.

5) We should agree to prepare the signals for these pins:

net unlock-settings                        => gmoccapy.unlock-settings

gmoccapy.warning-confirm IN
gmoccapy.blockdelete IN
gmoccapy.delete-message IN
gmoccapy.error OUT
gmoccapy.warning-confirm IN
h-button IN
v-button IN

Do you think that unnecessarily created signals canload the CPU in normal PC? If not I can suggest signal names. The user could then just use them in custom.hal. (custom.hal is an unoverwritten configuration file.)

c-morley commented 4 months ago

I am confused by c-morley too with his sentence aboat master. My picture above are from 2.9 branche. I dont use master branche.

Edit: I think I understand Chris. He wants this Pull Request to be in branch master not in 2.9. We should respect the wishes of Chris, as the author of PncConf.

I think I was confused too - I'm sorry. The code, as you said, is in 2.9 and master. I am open to updates to pncconf in 2.9

hansu commented 4 months ago
2. Make the gmoccapy_postgui.hal file rewritable by PncConf

done

3. Draw attention to this at the beginning of the file with the text:
# If you make changes to this file, they will be
# overwritten when you run PNCconf again

done

4. We should agree whether we will also add this:
net spindle-vel-cmd-rpm-abs        => gmoccapy.spindle_feedback_bar
net spindle-at-speed                     => gmoccapy.spindle_at_speed_led

I added net spindle-at-speed => gmoccapy.spindle_at_speed_led but for the other there is more to do than this single line, so I would not add this

I think this is ready to go, if you agree And furthermore I would not add some signals just for preparation.

zz912 commented 4 months ago

Something is wrong. Probably GIT problem.

I downloaded from github: https://github.com/hansu/linuxcnc/tree/issue-2820

There are not your the newest changes.

hansu commented 4 months ago

Haha sorry I forgot to commit :D

zz912 commented 4 months ago

I tested it. You can merge it. Thank you.