{"action":"created","issue":{"url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10688","repository_url":"https://api.github.com/repos/MarlinFirmware/Marlin","labels_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10688/labels{/name}","comments_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10688/comments","events_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10688/events","html_url":"https://github.com/MarlinFirmware/Marlin/pull/10688","id":322146242,"number":10688,"title":"[2.0.x] Refactor, optimization of core planner/stepper/endstops logic","user":{"login":"ejtagle","id":831601,"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":92052497,"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":378,"created_at":"2018-05-11T02:26:40Z","updated_at":"2018-05-20T09:25:29Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/MarlinFirmware/Marlin/pulls/10688","html_url":"https://github.com/MarlinFirmware/Marlin/pull/10688","diff_url":"https://github.com/MarlinFirmware/Marlin/pull/10688.diff","patch_url":"https://github.com/MarlinFirmware/Marlin/pull/10688.patch"},"body":"### Jitter reduction, pulse regularity\r\nMarlin, due to the way the Stepper ISR was written, was introducing severe jitter to the created stepper pulses. By \"jitter\" it is understood, once the Stepper ISR is fired, the amount of time elapsed from the start of the ISR (timer compare match) to the exact place in time where the stepper pulses are created varies wildly depending on several conditions of the preceding code of the ISR.\r\n\r\nThe other problem that also was causing jitter was that Marlin abused the use of Critical sections to protect against corruption of the variables of the Stepper and Planner. There were sections of the Planner that were running the Planner algorithm with interrupts disabled!! - Having interrupts disabled means the Stepper ISR does NOT execute on time, and the Stepper pulses will be delayed by all the duration of the Critical Section.\r\n\r\nThat will cause noises, and in extreme cases, could lead to lost steps. On interpolating drivers, such as TMC2xxx, the time interpolation algorithm that those drivers internally have create non uniform accelerations and deaccelerations between steps\r\n\r\nThe first step to try to reduce and/or eliminate jitter was to determine all the dependencies of the Planner and Stepper. Marlin had crossed references from the Planner to the Stepper and vice-versa, making extremely hard to determine that. \r\n\r\n### Refactor, division of Planner/Stepper\r\nSo, we refactored the Stepper and Planner classes in such a way that the Stepper does not need to know anything about the Planner (except the block_t structure), and Marlin does not have to know anything about the Stepper - All interactions with the Motion system are now handled by the Planner class.\r\n\r\nWith that refactoring done, now the structure is pretty clear: We first proceeded to reorder the stepper ISR routine in such a way that the first thing that is done was to create the step pulses, then after that time critical part, all the querying for new blocks (if needed) of the Planner is done.\r\n\r\n### Move endstop check to Temperature ISR\r\nAfter that, we realized that there was some things that can be offloaded from the Stepper ISR, specifically, the endstop processing can be fully run with no problems at all from the temperature ISR. So we also did that change - The Stepper ISR should be as simple as possible!!\r\n\r\nAfter removing the endstop processing from the Stepper ISR, there was no need for the ISR splitting. We reimplemented the ISR splitting in a much simpler way that works very well\r\n\r\n### Drop Planner Critical Blocks\r\nThe next thing was to remove all critical sections that are not required. The planner does NOT require a critical section to queue blocks, as we are using a circular FIFO queue. We can exploit that and avoid critical sections, thus reducing jitter.\r\n\r\nAnd, if a critical section is needed (to ensure atomic reads or modifications of structures), instead of globally disabling all the interrupts, we selectively disable the ISR that could interfere with the operation for the smallest period of time possible.\r\n\r\n### Fix Due ISR Test\r\nWe found a bug in the Due implementation on the macro used to determine if an ISR was enabled. It was always returning the interrupt as disabled. On Sam3x, there is an specific register to check if interrupts are enabled or not. It was not used, and we were reading a WRITE-ONLY register. That bug was fixed.\r\n\r\n### Optimize Priority Emulation\r\nThen, on AVR, another cause of ISR latency was the ISR priority emulation done in Software, between the Temperature ISR and the Stepper ISR. We wrote a custom assembly prologue and epilogue for those Stepper and Temperature ISRs, so we can do the emulation as fast as possible, reducing the latency of the interrupt processing from 60 to 12 cycles. \r\n\r\n### Due Emergency Parser\r\nWe added an Emergency parser to the Due CDC.\r\n\r\n### Optimize Planner maths\r\nThen we started Planner optimizations. Using speed squared saves TONS of SQRTs on the Planner algoritm, easily improving 10x its performance\r\n\r\n### Complete Junction Deviation\r\nWe have added all the Junction deviation features, as discussed in https://github.com/MarlinFirmware/Marlin/issues/10341#issuecomment-388191754 \r\n\r\n### Smarter Delays\r\nFinally, we factored out all delays into proper common routines by architecture (AVR/ARM). No more delays in NOPs. Now you can use DELAY_NS() and DELAY_US() and nops are calculated based on CPU speed (and also, uses loops instead of tons of NOPs). DELAY_NS() and DELAY_US() use busy loops, so are safe to be used even on ISRs\r\n\r\n### Optimization of Endstop handling, and avoiding polling as much as possible if interrupts on pin change are available.\r\n\r\n### Proper skipping of SYNC blocks, both in planner and in trapezoidal shape calculations\r\n\r\n### Alternative joining algorithm (this is under evaluation ... it ´s done and commited, but opinions are welcome)"},"comment":{"url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/comments/390468504","html_url":"https://github.com/MarlinFirmware/Marlin/pull/10688#issuecomment-390468504","issue_url":"https://api.github.com/repos/MarlinFirmware/Marlin/issues/10688","id":390468504,"user":{"login":"thinkyhead","id":698003,"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-05-20T09:25:29Z","updated_at":"2018-05-20T09:25:29Z","author_association":"MEMBER","body":"> In my humble opinion, the low level and core features / bugs / improvements directly linked to actually printing should have the highest priority\r\n\r\nYes, and in fact in the current run-up to the 1.1.9, for the last several weeks bug fixing is the main thing we have been focusing on, most especially the mysterious layer shifting issue which affect some users. That has been taking a lot of time to track down, and this PR seems to be helping, which is why I have wanted to get it merged as soon as possible.\r\n\r\nBut I also need to keep my issue list down to a manageable level, so I try to work on as many as I can each day to get them down under 40. Right now they're around 80 because I've been spending a lot of time on this one, which I consider the most valuable PR in a long time.\r\n\r\nIf you peek at the current list of open and un-merged PRs you'll see that we're holding back obscure and added features in favor of patches for outstanding issues, and looking through the commits you will find that the vast majority since 1.1.8 have focused on fixing bugs and (for 2.0.x) getting the HALs in more complete shape.\r\n\r\nWe really are working as hard as we can to make sure that this final 1.1.x release is as bug-free and improved as possible, and we are trying to be disciplined about it. At the same time I'm trying to discourage \"support issues\" where we help people figure out their configurations because these eat up a lot of our valuable development and PR review time. But they keep on coming, and we just try to clear them up quickly.\r\n\r\nAnyway, enough defensiveness from me! This is something I do out of a love and passion for the state of the art and for the love of software engineering. I've been enjoying this PR and have been learning a lot from it. I've wanted to spend much more time with it, but as mentioned there are a lot of feature requests and bug reports always piling up.\r\n\r\nI wish to keep this fun and enjoyable and light, to keep helping this project along. I don't like to get into personal animosity, and I have not set out to hurt or insult anybody. I really don't like being treated like a villain and made to feel like I'm \"a bad person\" when I know very damn well that I'm not."}}
{"actor":{"display_login":"thinkyhead"}}