Skip to content

docs - Correct the Python Interface example function ok_for_mdi#605

Merged
jethornton merged 1 commit intoLinuxCNC:2.8from
lorenzncode:doc-ok_for_mdi_2.8
Jun 9, 2019
Merged

docs - Correct the Python Interface example function ok_for_mdi#605
jethornton merged 1 commit intoLinuxCNC:2.8from
lorenzncode:doc-ok_for_mdi_2.8

Conversation

@lorenzncode
Copy link
Contributor

This PR corrects the logic in the ok_for_mdi() example function. I filed a separate request for 2.7.

…achine is homed before returning True. The homed attribute returns a tuple of length 9 with home status of each axis. The current example will always return True for the homed test even if all axes are unhomed.

Since there is a 1:1 correspondence between a joint and an axis, I suggest to correct the example by counting the homed axes and comparing with the number of joints (2.8) or number of axes (2.7):

2.8: s.homed.count(1) == s.joints

2.7: s.homed.count(1) == s.axes
@jethornton
Copy link
Collaborator

Both PR's are on Master and it looks like your repo was not up to date. You should git pull --rebase before submitting the PR and git checkout 2.7.

@lorenzncode lorenzncode changed the base branch from master to 2.8 June 9, 2019 11:58
@lorenzncode
Copy link
Contributor Author

I changed the base branch of the PR to 2.8. Is that better? If so I can try making the same change to the other PR that was intended for the 2.7 branch.

@lorenzncode
Copy link
Contributor Author

I filed the separate PR for 2.8 because the 2.7 example will not work in 2.8.

For example consider a JOINTS=4 machine with trivkins coordinates=XYYZ in 2.8.

s.axes in 2.8 will return 3
s.joints (new in 2.8) will return 4. This is what we need to compare with s.homed.count(1) for 2.8.

@jethornton jethornton merged commit 54541dd into LinuxCNC:2.8 Jun 9, 2019
@lorenzncode lorenzncode deleted the doc-ok_for_mdi_2.8 branch June 9, 2019 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants