Paciente8159 / uCNC

µCNC - Universal CNC firmware for microcontrollers
https://github.com/Paciente8159/uCNC/wiki
GNU General Public License v3.0
265 stars 60 forks source link

fix systems sync after HMap generation #680

Closed Paciente8159 closed 4 months ago

Paciente8159 commented 4 months ago
tomjnixon commented 4 months ago

Hi, thanks for looking at this. The new retraction behaviour seems better.

This doesn't fix the issue for me, though. As mentioned in the issue, after the parser calls mc_build_hmap, it updates parser_last_pos to target:

https://github.com/Paciente8159/uCNC/blob/857f96d47742e2b4a13ab5c4183eaf924576eb08/uCNC/src/core/parser.c#L1780

then:

https://github.com/Paciente8159/uCNC/blob/857f96d47742e2b4a13ab5c4183eaf924576eb08/uCNC/src/core/parser.c#L1806

So, the mc_sync_position call sets parser_last_pos correctly, then this is un-done, because mc_build_hmap leaves target set to a position beyond the probe grid.

tomjnixon commented 4 months ago

Ah, i missed that for G38 probing the parser returns before that memcpy, so maybe adding that (plus the sync you added) is the right solution:

https://github.com/Paciente8159/uCNC/blob/857f96d47742e2b4a13ab5c4183eaf924576eb08/uCNC/src/core/parser.c#L1775

Paciente8159 commented 4 months ago

Ah, i missed that for G38 probing the parser returns before that memcpy, so maybe adding that (plus the sync you added) is the right solution:

https://github.com/Paciente8159/uCNC/blob/857f96d47742e2b4a13ab5c4183eaf924576eb08/uCNC/src/core/parser.c#L1775

what about if you replace the break; at line #1795 by return error;

tomjnixon commented 4 months ago

Just adding that return doesn't do it, because mc_sync_position saves the current machine position, but the two mc_line calls to move the machine to the start position are still running.

With these two commits (in addition to yours) it works:

https://github.com/tomjnixon/uCNC/commit/50198779b20e8b8e6424e3f90a09679df962423c

https://github.com/tomjnixon/uCNC/commit/bb58f13580ddee2ac2a1904e2b2e4469becb5f7c

I'm not sure about the exact logic in the parser, it seems like it should return an error in case G39.1 etc, instead of doing nothing (and then saving the target).

Paciente8159 commented 4 months ago

Just adding that return doesn't do it, because mc_sync_position saves the current machine position, but the two mc_line calls to move the machine to the start position are still running.

With these two commits (in addition to yours) it works:

tomjnixon@5019877

tomjnixon@bb58f13

I'm not sure about the exact logic in the parser, it seems like it should return an error in case G39.1 etc, instead of doing nothing (and then saving the target).

Ah that. Ok. Then before the mc_sync_position I need to call the itp_sync(). This will wait for the motion to complete before updating the position. I forgot to add that as this should be a synched motion like probing.