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

multi spindle discussion #713

Closed
c-morley opened this issue Apr 8, 2020 · 39 comments
Closed

multi spindle discussion #713

c-morley opened this issue Apr 8, 2020 · 39 comments

Comments

@c-morley
Copy link
Collaborator

c-morley commented Apr 8, 2020

Current behavior AFAIKT with a bit of testing.

M3/M4 will start all spindles as long as their respective S codes are already set, adding a $ with a number will start a specific spindle.
M5 stops all spindles - adding a $ with a number specifies which spindle to stop
After an s code has used a $ to specify a spindle, plain s codes after that will only control that specified spindle.
You cannot put a $ with a number with out another spindle control code -ie you can't preset a specific spindle to be modal in this way.

My thoughts - Mostly copied from dev email list...

Setting the s code to a particular spindle (with %) will from then on always controls that spindle with a plain s code. Not sure if that is on purpose but i wouldn't think so.

If you don't set the s code with $ then only spindle 0 will start and stop with m3 m4 m5.
meaning a program running on a single spindle machine runs the same on a dual spindle machine.
But that relying on not setting the $ code seems a bit risky considering linuxcnc can be remapped and use subroutines (that could set it) or maybe left over from a manual setting (I didn't try that)

I did find that using the python module to control the spindle (like all the python screens do) the stop command seems to assume spindle 0 so pressing stop button only stops spindle 0. Which may or may not be whats expected but there is no python command to stop all spindles, unless you added one I didn't check.

I still feel that with mcode and python module, specifying all spindles with say -1 is a better practice.
We should fix the python module to be able to specify all spindles for stopping.

I feel that with s code, it's probably a bug that it remembers the last specified spindle and continues to adjust it with a plain s code.
It would be good practice and consistent if specifying $-1 would set the spindle speed for all spindles.

@andypugh
Copy link
Collaborator

andypugh commented Apr 8, 2020

Making S modal was deliberate, but on balance I think it was a mistake.

It is probably also true that if someone really needed to set up a group of spindles and start / stop them together then they could be expected to read the manual to find out how.

statetags and various Python interfaces do not, at the moment, report on more than the default spindle. The overlap between those using these interfaces and those using multiple spindles is probably small.

@andypugh
Copy link
Collaborator

andypugh commented Apr 8, 2020

M19 stops all spindles regardless of $ number. Should it?

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

m19 - I wouldn't think so.
Yes I thought maybe it was modal so tried $1 on a line by it's self and that was not accepted. I agree I don't think that is the way to go.

Qtvcp now has support for multiple spindles if some one wants to add them.

I just think it's important to iron out the details before releasing so we don;t have to break behavior on the next release.

andypugh added a commit that referenced this issue Apr 8, 2020
Without a $ M3, M4 & M5 now act on spindle zero.
To act on all spindles use $-1

Signed-off-by: andypugh <andy@bodgesoc.org>
@andypugh
Copy link
Collaborator

andypugh commented Apr 8, 2020

See 3139328

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

oh great I'll test that out.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

That is a great improvement.
s code will not accept $-1 - I think it should.
I'll see if I can figure out how to get python to do the right thing with -1 command.

One other question.
I was entering commands in MDI and if I entered a S or M3/M5 wrong it stopped all spindles.
Do you think it should?
I think it should do nothing other then complain.

@andypugh
Copy link
Collaborator

andypugh commented Apr 8, 2020

s code will not accept $-1 - I think it should.

I suppose that wanting all spindles at the same speed is quite likely on multi-spindle (copy) routers and dual-spindle lathes.

I was entering commands in MDI and if I entered a S or M3/M5 wrong it stopped all spindles.
Do you think it should?

Probably not, but that is built-in error behaviour.
(And if you ignore the error and type in the feed move anyway, you deserve all you get)

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

Well It means if you set all your spindles speeds up and then go to adjust one but get it wrong it turns them all off and you need to set them again. you don;t deserve that :)

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

So it looks like there is a NML message send for each spindle to control ie there is no message to control all of the spindles at once.
So to fix up python I need it to send a message per spindle if the user specifies -1

Does that sound right to you?

@andypugh
Copy link
Collaborator

andypugh commented Apr 8, 2020

But, short of not doing the check, I don't see a way to change that.

@andypugh
Copy link
Collaborator

andypugh commented Apr 8, 2020

Yes, it's a message-per-spindle.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

ok thanks for confirming. I'll work on the python code

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

Hit a snag - python doesn't know how many spindles are available. If I send a command for the max amount of spindles, motion gives error on the ones not available.

I guess we could silently ignore the unavailable spindle commands?
Other suggestions?

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 8, 2020

Or maybe I should forget it and document that in python there is no 'all spindles' command

@andypugh
Copy link
Collaborator

andypugh commented Apr 9, 2020

Does Python have a way of finding it out? It seems like it might be a useful thing to know.
It's a bit of a mess anyway, as (like in a couple of other places) the motmod modparam num_spindles and INI NUM_SPINDLES have to match, but the two halves of LinuxCNC can't see each other to check this.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

status reports the number of spindles and the python module can reads that.
but it doesn't automatically. and i think it would be painful to make it do so.
it would seem that maybe the motion component should know how many spindle there are and then you could just tell it to stop all the spindle etc. - maybe i'll look into that possibility.
it seems that motion and the interpreter don't have anyway to know what each other knows about spindle numbers.

@andypugh
Copy link
Collaborator

andypugh commented Apr 9, 2020

Just document that Python can't do it, and move on then.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

https://github.com/LinuxCNC/linuxcnc/commits/motion-all-spindles-command
This teaches motion that -1 means all spindles, so now python can send -1 to command all spindles.
please take a look when you have time.

@andypugh
Copy link
Collaborator

andypugh commented Apr 9, 2020

Is this compatible with the existing stuff? (including da3c1c1 )

I am a bit nervous of the big block of duplicated code. How about:
int s0 = spindle;
int s1 = spindle;
if (spindle = -1) { 
    s0 = 0;
    s1 = settings->num spindles - 1;
}
for (i = s0; i <= s1; i++){
.... one common block of code to maintain....
}

edit: I kept considering doing this with the other changes for -1, but thought that in that case clarity of code just won out.
edit2: it needs to be <= in the for loop.
edit3: but then we need to set s1 one less than spindles.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

yes I didn't like it either, but it was clear. I can add a comment to describe the somewhat strange code.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

meaning I'll use your changes with a comment

@andypugh
Copy link
Collaborator

andypugh commented Apr 9, 2020

I am not clear if this replaces the changes I made? I think it does?

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

It doesn't replace your code. But probably you could change the code to send one NML message using -1 as the spindle number rather then several.
Also still need to modify s code to use %-1

@andypugh
Copy link
Collaborator

andypugh commented Apr 9, 2020

I did S several minutes ago: da3c1c1

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

ok I redid it as you asked and tested with my system a bit. please take a look.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

ha hard to keep up :) i'll pull it in and test.

@c-morley
Copy link
Collaborator Author

c-morley commented Apr 9, 2020

all seems to work fine !

@andypugh
Copy link
Collaborator

I tried to pass a -1 for all spindles deeper in to the depths:
4b7b487
But it doesn't work at all. (whereas prior to that commit is was all nice)

emccanon uses the spindle number as an index into the spindles array:
https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/task/emccanon.cc#L1888
So I would need a loop in there for spindle == -1, and I would rather have the changes in interp not canon.

@c-morley
Copy link
Collaborator Author

fair enough - so far having multiple motion messages is not anything anyone is complaining about.
I'm not sure if single stepping, steps through each spindle message or ignores them completely.
that's the only possible difference I can think of at the moment.

Are you happy with the motion code, so I can commit ?

@andypugh
Copy link
Collaborator

Already committed.

Next question: CSS with more than one spindle? I reckon you could.

@c-morley
Copy link
Collaborator Author

Ya I guess that could be possible.
i could see them slaved to different axes too. - just gets deeper doesn't it
But maybe we should cross that bridge later.

@andypugh
Copy link
Collaborator

CSS is explicitly slaved to X.
Which seems like missing a trick, it could work on cutter diameter for mills....

@c-morley
Copy link
Collaborator Author

yes on a mill with a sub spindle on the bed, it might be desirable to slave the sub spindle speed.to Z position

@c-morley
Copy link
Collaborator Author

I modified my second spindle (which is table mounted spindle) from velocity to positional mode.
So now it's an indexer. Running by using the S code as the positional request in degrees.
This was easy to set up and works well. I do notice it you ask for S0 it cancels spindle enable.
Does this really need to happen? I can't see with a regular spindle expecting S0 would turn off the spindle, I would expect it to set the requested RPM to 0. There is not much difference in a velocity spindle but a big difference in a positional spindle.Thoughts?

@andypugh
Copy link
Collaborator

I can't help feeling that using S to control position is a bit eccentric when LinuxCNC supports 3 rotary axes.

S = 0 has always turned off the spindle in LinuxCNC. It seems a bit of a big step to change that.

@c-morley
Copy link
Collaborator Author

I'm using it as an indexer not a rotary axis - motion does not manage the indexer.
It's much easier to add to the config and it seems to work well - the S0 is an annoyance in this sistuation.

I know S0 has always done this, but I'm suggesting maybe people relying on this behavior is much rarer (I can't think why you would) and this is a major release so changing is an option.

@c-morley
Copy link
Collaborator Author

To put this another way;
If one expects that S0 automatically adds a M5 (which it basically is doing), then why does issuing S1000 not automatically issue M3 or S-1000 not automatically issue M4. (and I am not wanting this - it's just for discussion)

I've never seen a gcode program like:
M3 S100

S0
M2

It's another case for being explicit - S code is for adjustment, M is for state
Thanks for considering.

@andypugh
Copy link
Collaborator

I am inclined to leave this as it is now. Maybe think about changing for 2.9, but it might make sense to leave the current slightly odd behaviour consistent.

@c-morley
Copy link
Collaborator Author

yes I agree - I have a patch for master that needs a little more testing but so far works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants