{"action":"created","issue":{"url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10911","repository_url":"https://api.github.com/repos/MarlinFirmware/Marlin","labels_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10911/labels{/name}","comments_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10911/comments","events_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10911/events","html_url":"https://github.com/MarlinFirmware/Marlin/pull/10911","id":328362014,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkxOTIxNTI3","number":10911,"title":"[2.0.x] Small assorted collection of fixes and improvements","user":{"login":"ejtagle","id":831601,"node_id":"MDQ6VXNlcjgzMTYwMQ==","avatar_url":"https://avatars2.githubusercontent.com/u/831601?v=4","gravatar_id":"","url":"https://api.github.com/users/ejtagle","html_url":"https://github.com/ejtagle","followers_url":"https://api.github.com/users/ejtagle/followers","following_url":"https://api.github.com/users/ejtagle/following{/other_user}","gists_url":"https://api.github.com/users/ejtagle/gists{/gist_id}","starred_url":"https://api.github.com/users/ejtagle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ejtagle/subscriptions","organizations_url":"https://api.github.com/users/ejtagle/orgs","repos_url":"https://api.github.com/users/ejtagle/repos","events_url":"https://api.github.com/users/ejtagle/events{/privacy}","received_events_url":"https://api.github.com/users/ejtagle/received_events","type":"User","site_admin":false},"labels":[{"id":196934261,"node_id":"MDU6TGFiZWwxOTY5MzQyNjE=","url":"https://api.github.com/repos/MarlinFirmware/Marlin/labels/PR:%20Bug%20Fix","name":"PR: Bug Fix","color":"bfe5bf","default":false},{"id":92052497,"node_id":"MDU6TGFiZWw5MjA1MjQ5Nw==","url":"https://api.github.com/repos/MarlinFirmware/Marlin/labels/PR:%20Improvement","name":"PR: Improvement","color":"c2e0c6","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2018-06-01T02:49:12Z","updated_at":"2018-06-01T23:11:41Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/MarlinFirmware/Marlin/pulls/10911","html_url":"https://github.com/MarlinFirmware/Marlin/pull/10911","diff_url":"https://github.com/MarlinFirmware/Marlin/pull/10911.diff","patch_url":"https://github.com/MarlinFirmware/Marlin/pull/10911.patch"},"body":"- Get rid of mostly all critical sections on the Serial port drivers for AVR and DUE: We are using FIFOs, and proper usage of those FIFOs allow us to run with interrupts enabled the queuing and dequeuing of items.\r\nAlso, if using 8bit indices (for AVR) or 8 up to 32bit indices (for ARM), there is no need to protect reads and writes to those indices.\r\n- Simplify the XON/XOFF logic quite a bit. Much cleaner now (both for AVR and ARM)\r\n- Prevent the race condition (edge case) that could happen when estimating the proper value for the stepper timer (by reading it) and writing the calculated value for the time to the next ISR by disabling interrupts in those critical and small sections of the code - The problem could lead to lost steps.\r\n- Fix the dual endstops not properly homing bug #10897 (or at least this is what i think. I have no actual printer of this kind to test it)\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/comments/394032519","html_url":"https://github.com/MarlinFirmware/Marlin/pull/10911#issuecomment-394032519","issue_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10911","id":394032519,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NDAzMjUxOQ==","user":{"login":"thinkyhead","id":698003,"node_id":"MDQ6VXNlcjY5ODAwMw==","avatar_url":"https://avatars2.githubusercontent.com/u/698003?v=4","gravatar_id":"","url":"https://api.github.com/users/thinkyhead","html_url":"https://github.com/thinkyhead","followers_url":"https://api.github.com/users/thinkyhead/followers","following_url":"https://api.github.com/users/thinkyhead/following{/other_user}","gists_url":"https://api.github.com/users/thinkyhead/gists{/gist_id}","starred_url":"https://api.github.com/users/thinkyhead/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thinkyhead/subscriptions","organizations_url":"https://api.github.com/users/thinkyhead/orgs","repos_url":"https://api.github.com/users/thinkyhead/repos","events_url":"https://api.github.com/users/thinkyhead/events{/privacy}","received_events_url":"https://api.github.com/users/thinkyhead/received_events","type":"User","site_admin":false},"created_at":"2018-06-01T23:11:41Z","updated_at":"2018-06-01T23:11:41Z","author_association":"MEMBER","body":"Since the planner/stepper refactor we've had an increasing number of reports of \"Probing failed\" lately, and I've confirmed that this bug exists in some of my testing. When I went to track it down I found that sometimes a probe move would stop early without triggering the probe, even though it should have gone well below the point where it was stopping. This seemed more common when doing double or multiple-probing at each point.\r\n\r\nI haven't completely tracked it down, but my hypothesis is that it's related to `do_probe_move` failing to update the planner with the current position after the probe move has completed. This would be consistent with the probe stopping early, since if a previous probe move stopped at Z=0 but the planner still thought it was at Z=-10, then on the next probe move the planner would stop early.\r\n\r\nThe `set_current_from_steppers_for_axis` and `SYNC_PLAN_POSITION_KINEMATIC` calls at the end of `do_probe_move` _should_ ensure that the planner and the coordinate system both know the real physical position of the Z axis when the probe triggered. Of course, this depends on the correct behavior of the stepper ISR — first, that it should stop on a probe or endstop trigger, and second, that it should freeze its `count` value at that point.\r\n\r\nI'll continue my testing and try to to track down the exact reason why the probe move is stopping early. So far my best guess is that the planner is not being corrected after the probe move stops, thus leading to these moves stopping short of the probe triggering.\r\n\r\nThinking it might be due to a race condition, I've added a commit to this PR and its companion to set the stepper positions right away on the call to `planner._set_position_mm` when there are no blocks in the planner queue, rather than using a \"sync\" block to update the stepper counts \"soon.\" I don't have high confidence that this will actually help with the \"Probing failed\" bug, since that mainly depends on the planner's positions and not the stepper counts, but it seemed like a good idea nevertheless, to make sure that `set_current_from_steppers_for_axis` can be used right away after calling `planner.set_position_mm` without needing to also call `stepper.synchronize`."}}
{"actor":{"display_login":"thinkyhead"}}