{"action":"created","issue":{"url":"https://api.github.com/repos/MaslowCNC/Firmware/issues/467","repository_url":"https://api.github.com/repos/MaslowCNC/Firmware","labels_url":"https://api.github.com/repos/MaslowCNC/Firmware/issues/467/labels{/name}","comments_url":"https://api.github.com/repos/MaslowCNC/Firmware/issues/467/comments","events_url":"https://api.github.com/repos/MaslowCNC/Firmware/issues/467/events","html_url":"https://github.com/MaslowCNC/Firmware/pull/467","id":370438724,"node_id":"MDExOlB1bGxSZXF1ZXN0MjIzMTE0NDc3","number":467,"title":"Beam tilt correction","user":{"login":"johnboiles","id":218876,"node_id":"MDQ6VXNlcjIxODg3Ng==","avatar_url":"https://avatars3.githubusercontent.com/u/218876?v=4","gravatar_id":"","url":"https://api.github.com/users/johnboiles","html_url":"https://github.com/johnboiles","followers_url":"https://api.github.com/users/johnboiles/followers","following_url":"https://api.github.com/users/johnboiles/following{/other_user}","gists_url":"https://api.github.com/users/johnboiles/gists{/gist_id}","starred_url":"https://api.github.com/users/johnboiles/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johnboiles/subscriptions","organizations_url":"https://api.github.com/users/johnboiles/orgs","repos_url":"https://api.github.com/users/johnboiles/repos","events_url":"https://api.github.com/users/johnboiles/events{/privacy}","received_events_url":"https://api.github.com/users/johnboiles/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2018-10-16T04:51:34Z","updated_at":"2018-10-16T15:29:20Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/MaslowCNC/Firmware/pulls/467","html_url":"https://github.com/MaslowCNC/Firmware/pull/467","diff_url":"https://github.com/MaslowCNC/Firmware/pull/467.diff","patch_url":"https://github.com/MaslowCNC/Firmware/pull/467.patch"},"body":"I'm starting to cherry-pick features and fixes from @madgrizzle's fork so that we can get them merged back upstream.\r\n\r\nThis change adds beam tilt correction -- From reading the code, it adjusts the x,y coordinates of the motors to account for the top beam being out-of-level.\r\n\r\nRelated GroundControl change is in https://github.com/MaslowCNC/GroundControl/pull/778"},"comment":{"url":"https://api.github.com/repos/MaslowCNC/Firmware/issues/comments/430283379","html_url":"https://github.com/MaslowCNC/Firmware/pull/467#issuecomment-430283379","issue_url":"https://api.github.com/repos/MaslowCNC/Firmware/issues/467","id":430283379,"node_id":"MDEyOklzc3VlQ29tbWVudDQzMDI4MzM3OQ==","user":{"login":"johnboiles","id":218876,"node_id":"MDQ6VXNlcjIxODg3Ng==","avatar_url":"https://avatars3.githubusercontent.com/u/218876?v=4","gravatar_id":"","url":"https://api.github.com/users/johnboiles","html_url":"https://github.com/johnboiles","followers_url":"https://api.github.com/users/johnboiles/followers","following_url":"https://api.github.com/users/johnboiles/following{/other_user}","gists_url":"https://api.github.com/users/johnboiles/gists{/gist_id}","starred_url":"https://api.github.com/users/johnboiles/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johnboiles/subscriptions","organizations_url":"https://api.github.com/users/johnboiles/orgs","repos_url":"https://api.github.com/users/johnboiles/repos","events_url":"https://api.github.com/users/johnboiles/events{/privacy}","received_events_url":"https://api.github.com/users/johnboiles/received_events","type":"User","site_admin":false},"created_at":"2018-10-16T15:29:20Z","updated_at":"2018-10-16T15:29:20Z","author_association":"NONE","body":"> I think we should hold off on this for a bit, we need to be simplifying the math\r\nto figure things out, not add more variation.\r\n\r\nI'll leave it to you and @madgrizzle to debate the value of beam tilt correction. In my opinion it can't hurt. It seems like a great experimental feature that the community should continue to explore. I think there's value in making experimental features easier to try for people that don't feel comfortable running from source.\r\n\r\nWith regards to simplification, I think the change here that breaks out separate variables for `leftMotorX`, `leftMotorY`, `rightMotorX`, `rightMotorY` _is_ a simplification. In the existing code, the `_xCordOfMotor` variable really means more of 'x distance from the center' and it's then multiplied by `-1` to get get the actual x coordinate of the left motor. Even if we decide not to merge top beam correction, what do you think about keeping the separate `leftMotorX`, `leftMotorY`, `rightMotorX`, `rightMotorY` variables? I think it's overall more readable. To make the existing code even more readable we could separate the top beam correction into separate local variables\r\n\r\n```\r\nfloat topBeamTiltXCorrectionFactor = cos(sysSettings.topBeamTilt*0.0174532925199433);\r\nfloat topBeamTiltYCorrectionFactor = sin(sysSettings.topBeamTilt*0.0174532925199433);\r\n\r\n    leftMotorX = topBeamTiltXCorrectionFactor*sysSettings.distBetweenMotors/-2.0;\r\n    leftMotorY = topBeamTiltYCorrectionFactor*sysSettings.distBetweenMotors/-2.0 + (sysSettings.motorOffsetY+sysSettings.machineHeight/2.0);\r\n    rightMotorX = topBeamTiltXCorrectionFactor*sysSettings.distBetweenMotors+leftMotorX;\r\n    rightMotorY = topBeamTiltYCorrectionFactor*sysSettings.distBetweenMotors/2.0 + (sysSettings.motorOffsetY+sysSettings.machineHeight/2.0);"}}
{"actor":{"display_login":"johnboiles"}}