Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit Conversion Broken #49

Open
tedh-shopbot opened this issue Mar 12, 2022 · 6 comments
Open

Unit Conversion Broken #49

tedh-shopbot opened this issue Mar 12, 2022 · 6 comments
Assignees
Labels
bug priority_1_Top severity_2_BrokenFeature A single feature does not work, must be fixed to be used

Comments

@tedh-shopbot
Copy link
Contributor

The primary/basic unit conversion from inches to mm or mm to inches no longer updates the current position {pos: } after the unit change made with {gun: }.

This functionality worked correctly through version 101.57.32.
It was broken with commit 4d149970 "main item 34 faulty sr from feedhold or kill" 1/28/22 and remains broken through 101.57.35.

Sample after break (edited for readibility):
g2: --S-1647109441151----> {gun:n} [g2]
g2: <-S-1647109441158----- {"r":{"gun":0},"f":[1,0,9]} [g2]
g2: --S-1647109392429----> {mpo:n} [g2]
g2: <-S-1647109392440----- {"r":{"mpo":{"x":54.86393,"y":27.43183,"z":0,"a":0,"b":0,"c":0}},"f":[1,0,9]} [g2]
g2: --S-1647109418224----> {pos:n} [g2]
g2: <-S-1647109418235----- {"r":{"pos":{"x":2.16,"y":1.07999,"z":0,"a":0,"b":0,"c":0}},"f":[1,0,9]} [g2]
g2: --S-1647109452416----> {gun:1} [g2]
g2: <-S-1647109452423----- {"r":{"gun":1},"f":[1,0,9]} [g2]
g2: --S-1647109465315----> {mpo:n} [g2]
g2: <-S-1647109465325----- {"r":{"mpo":{"x":54.86393,"y":27.43183,"z":0,"a":0,"b":0,"c":0}},"f":[1,0,9]} [g2]
g2: --S-1647109472507----> {pos:n} [g2]
g2: <-S-1647109472520----- {"r":{"pos":{"x":2.16,"y":1.07999,"z":0,"a":0,"b":0,"c":0}},"f":[1,0,9]} [g2]
=> tool is now in metric and these values should reflect the change rather than just duplicating the inch values

(can't really call this a regression since it has always worked and is pretty basic)

(previously this was FabMo-Engine issue #827)

@tedh-shopbot tedh-shopbot added bug severity_2_BrokenFeature A single feature does not work, must be fixed to be used priority_1_Top labels Mar 12, 2022
@Cosmic7373
Copy link
Collaborator

Is this the kind of behavior that is expected, but is not occurring?

g21
{"r":{},"f":[1,0,4]}
{"sr":{"posx":76.2,"stat":3,"unit":1}}
g20
{"r":{},"f":[1,0,4]}
{"sr":{"posx":3,"stat":3,"unit":0}}

@tedh-shopbot
Copy link
Contributor Author

Yes.
FabMo uses gun:0/1 to set default units as well as the g20/21 which is run at the beginning of a file to specify the unit for the file.

@tedh-shopbot
Copy link
Contributor Author

tedh-shopbot commented Mar 13, 2022

With respect to 'gun', FabMo-G2core operates differently than G2core. That's because we needed/wanted a system for setting a default and persistent Unit mode independently of running a gcode file. I believe this is basically accomplished in:
canonical_machine.cpp Lines: 2516 - 2525

It has worked pretty well up til now.

@RobMackie RobMackie assigned Cosmic7373 and msxmatt and unassigned Cosmic7373 Mar 22, 2022
@tedh-shopbot
Copy link
Contributor Author

Hmm ...
Just spent some time in distraction today because this bug still exists and I had forgotten I had already documented it. It is pretty ugly. Definitely one we can not release without fixing.

@msxmatt
Copy link
Contributor

msxmatt commented Jul 11, 2022

Hey @tedh-shopbot yea it's been a little while, but I remember it was a bit ugly. I think we discussed pulling the out the faulty SR fix, since it appeared to break a number of items. We did some squashing of bugs for a while, but others cropped up so it seemed like that fix should be pulled for a release.

I can hop back on the chat and we can discuss how you'd like to attack it.

@tedh-shopbot
Copy link
Contributor Author

Thanks, Matt @msxconsulting. @RobMackie is pondering this one and a few other things. I'll let him get back to you on this one as he sorts plans out. =ted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug priority_1_Top severity_2_BrokenFeature A single feature does not work, must be fixed to be used
Projects
None yet
Development

No branches or pull requests

4 participants