MaslowCNC / GroundControl

This is the Ground Control software used to control the Maslow CNC Machine
https://www.MaslowCNC.com
GNU General Public License v3.0
275 stars 122 forks source link

Update background image feature messes up with home definition. #762

Open CptanPanic opened 5 years ago

CptanPanic commented 5 years ago

If you have home set using "Define home" button, when you click update background image, the gcode moves seemingly 2x farther from machine center. This is with latest master.

BarbourSmith commented 5 years ago

I was unable to replicate this one. Are you still seeing the same behavior? Can you give me steps to make it happen?

iceblu3710 commented 5 years ago

I am looking into this. I can only re-create this when using the "hold pick" dialog to move home. The error is proportional to the distance from the currently reported home as shown by the red cross-hairs.

For example: Load a 100mm square at (0,0). Use the hold pick menu to move home to the (0,100) position. Go to the background menu and press update background. The 100mm square's home will now be set to (0,100).

I think the Gcode drawing code re-draws with an offset from home. This way when I call the existing routines to update the background it offsets AGAIN.

I am looking into a few options but won't be able to dedicate a lot of time until the weekend. May I ask why you where using the update background button? I left it in from testing but never thought it would be used.

CptanPanic commented 5 years ago

@iceblu3710 I figured that update background is the button you pressed to update what gets saved as background. But honestly I have no idea what any of the buttons are supposed to do, so I clicked it, and that is when I noticed that my gcode had suddlenly been moved off the screen.

iceblu3710 commented 5 years ago

I will add an entry to the Wiki on my ToDo list.

Once you pick an image and alignment points you should never need to manually update the background. If you load a new image it will go to the alignment screen automatically. I used the button in dev when I was manually changing variables and needed to call the update routine.

BarbourSmith commented 5 years ago

A simple solution could be to simply take out the button then, right? If we don't need it it's just clutter

blurfl commented 5 years ago

I've re-created this issue on Windows and macOS, both by moving the sled using the arrows to re-define Home and by editing the 'Home Poisition X- and/or Y-Coordinate' value.

After changing Home, using Background makes the cut pattern move. Using Background/RemoveBackground leaves the file mis-rendered after the background image is removed. FWIW, repeating the pick Background/UpdateBackground to see the cut pattern move even more.

Using the Actions/UpdateGcode button re-renders the file correctly, with or without a background.

This file Lake border - Copy.nc.zip is interesting in that part of it is offset more than the rest.

iceblu3710 commented 5 years ago

The halfsies offsets I saw as well which lead me to believe it is a drawing offset reference issue that may be indicative of a bug in the code rendering. My only modifications to the core program was the background is dawn as a separate rectangle on the canvas.

As a quick fix, Tonight I will look into calling the Actions/UpdateGcode button after any background actions.

I will also remove the button as it is pretty useless now.

blurfl commented 5 years ago

If it's a clue, I find that even with no background loaded, the Background/RemoveBackground button offsets the render. Something in processBackground() - the way it calls updateGcode()? Does it need to set some args before the call?

BarbourSmith commented 5 years ago

I think I remember a similar issue I had at one point where pressing the "define home" button repeatedly would cause the code to incrementally move. Let me see if a little digging refreshes my memory as to why that was

Edit: I'm not seeing an obvious cause, but here are my thoughts

355 in frontPage.py sets the new location:

def moveOrigin(self):
    '''

    Move the gcode origin to the current location

    '''
    self.data.gcodeShift = [self.numericalPosX,self.numericalPosY]
    self.data.config.set('Advanced Settings', 'homeX', str(self.numericalPosX))
    self.data.config.set('Advanced Settings', 'homeY', str(self.numericalPosY))

Which triggers a callback set on line 59 of gcodeCanvas.py:

self.data.bind(gcodeShift = self.reloadGcode)              #No need to reload if the origin is changed, just clear and redraw`

Which then redraws the gcode with the new offset applied in the function moveLine() . I can imagine if something was redrawn multiple times the shift would be applied with each refresh so the drawn gcode would incrementally move

iceblu3710 commented 5 years ago

It was a simple copy paste error. I copied the re-draw code from above which was updateGcode instead of reloadGcode which is what I meant.

gecodeCanvas.py#L58self.data.bind(backgroundRedraw = self.reloadGcode)

blurfl commented 5 years ago

Good find! 💯🎉 That seems to fix what I was seeing.

iceblu3710 commented 5 years ago

PR #765

BarbourSmith commented 5 years ago

Excellent! Great work everyone!