Added "M4" for spindles

[bdmfab]

First pull request so I apologize for mistakes.

I have added M4 (spindle reverse) functionality for CNC spindles. This will set an output when calling M4 for drives that use this.

Added optional config parameter spindle.reverse_dir_pin to PWM and Analog modules. Can be defined as any output capable pin and "!" inverted if necessary.

Cloned the PWMSpindleControl module into EncoderSpindleControl adding the following config parameters:

Added type - encoder
spindle.reverse_dir_pin (Optional default=nc) Pin to reverse direction of spindle
spindle.switch_on_pin (Optional default=nc ) Pin to "Enable" drive when M3 or M4 is called
spindle.default_rpm (Optional default=10% of max) Min starting rpm to overcome friction
spindle.max_rpm (default=10000) Max rpm of spindle when PWM is 100%

This is for ongoing development as I will make considerable changes to this module.

[arthurwolf]

I do not know if the code is worth merging ( I just mean I haven't looked at it so I don't know if it matches the defined standard ), but I wanted to chime in to say this is a frequently requested feature, and one i need for work / was planning on implementing soon.

[wolfmanjm]

FYI I won't be considering this for merging until it is complete and has been fully tested by existing users of the spindle module.

Thank you.

[arthurwolf]

I should need to test it in a few weeks for work, I'll report here when that happens. In the meantime this is probably worth adding to the third-party-branches wiki page, will do.

…

On Thu, Jul 23, 2020 at 10:45 PM Jim Morris ***@***.***> wrote: FYI I won't be considering this for merging until it is complete and has been fully tested by existing users of the spindle module. Thank you. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1476 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2SFONKKNTWELNRQJ45BLR5COPDANCNFSM4PGCU2PQ> .

-- 勇気とユーモア

[bdmfab]

It should be completely backward compatible and not affect existing configs. It will become necessary when I complete the encoder module code in the long run as my goal is to tap, My plan is to implement the QEI hardware interface as well as software encoders. Quick question - I am creating an encoder module under tools, how do I make that build with CNC=1 like the spindle module. Thanks, Dan

…

On Thu, Jul 23, 2020 at 4:30 PM Arthur Wolf ***@***.***> wrote: I do not know if the code is worth merging, but I wanted to chime in to say this is a frequently requested feature, and one i need for work / was planning on implementing soon. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1476 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIU72TQ36TUK5TO7UIVK5J3R5CMX5ANCNFSM4PGCU2PQ> .

-- Thanks, Dan

[bdmfab]

I agree, when you say complete, have I missed something?

…

On Thu, Jul 23, 2020 at 4:45 PM Jim Morris ***@***.***> wrote: FYI I won't be considering this for merging until it is complete and has been fully tested by existing users of the spindle module. Thank you. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1476 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIU72TTTB2EZH5L2FO2KMSDR5COPDANCNFSM4PGCU2PQ> .

-- Thanks, Dan

[wolfmanjm]

You said you will be making "considerable changes" to the module. This creates a huge amount of work for me having to check every time you check in a change, which I am not prepared to do at the moment. So once you have finsihed making the considerable changes we will then consider the final result.
As for the encoder module if it is only used by the spindle then put it under the spindle and don't add yet another module that just makes things too complex.

[arthurwolf]

https://github.com/Smoothieware/Smoothieware/blob/edge/src/makefile#L73

…

On Thu, Jul 23, 2020 at 10:50 PM bdmfab ***@***.***> wrote: It should be completely backward compatible and not affect existing configs. It will become necessary when I complete the encoder module code in the long run as my goal is to tap, My plan is to implement the QEI hardware interface as well as software encoders. Quick question - I am creating an encoder module under tools, how do I make that build with CNC=1 like the spindle module. Thanks, Dan On Thu, Jul 23, 2020 at 4:30 PM Arthur Wolf ***@***.***> wrote: > I do not know if the code is worth merging, but I wanted to chime in to > say this is a frequently requested feature, and one i need for work / was > planning on implementing soon. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > < #1476 (comment) >, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AIU72TQ36TUK5TO7UIVK5J3R5CMX5ANCNFSM4PGCU2PQ > > . > -- Thanks, Dan — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1476 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2SFI6EZAS7WFK3MYWYCTR5CPAXANCNFSM4PGCU2PQ> .

-- 勇気とユーモア

[wolfmanjm]

Be aware that If you modify the makefile it will almost certainly make it impossible to merge later.

[bdmfab]

Sorry trying to stick to the contribution guidelines. The changes are not related to the M4 they are related to ongoing work on the encoder interface which will change it significantly. "As for the encoder module if it is only used by the spindle then put it under the spindle and don't add yet another module that just makes things too complex." It is used by the spindle as well as other elements such as slaved to an axis when performing synchronous motion and should be in its own module

…

On Thu, Jul 23, 2020 at 4:56 PM Jim Morris ***@***.***> wrote: You said you will be making "considerable changes" to the module. This creates a huge amount of work for me having to check every time you check in a change, which I am not prepared to do at the moment. So once you have finsihed making the considerable changes we will then consider the final result. As for the encoder module if it is only used by the spindle then put it under the spindle and don't add yet another module that just makes things too complex. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1476 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIU72TUYYNKH7V7A2FXZBNDR5CPWZANCNFSM4PGCU2PQ> .

-- Thanks, Dan

[wolfmanjm]

The problem is the two will rely on each other and you cannot have two outstanding PRs that rely on each other.

Also if something is needed by the core robot code then it cannot be a module.

[wolfmanjm]

FWIW this can also be simply done with a switch module that sets a pin on the M5 command. On my VFD for instance it would just set the pin for reverse.
As this has been dormant for 3 years now I am closing it. I will reopen if any further progress is made.