Added more flexible VFD support

[andrewmarles]

I'm creating this pull request in case you find this useful for the project. All of the new VFD code has been tested on actual machines, and at least one user has already got the MODVFD module working with an allen-bradley VFD via the custom registers. That said, I don't want to create an additional support burden for the project. But I do think that expanding the VFD support for GRBLHAL is a useful thing so I wanted to pass this stuff upstream.

In addition to the changes in the plugin, I added the following to settings.h in the core.

    //currently below are used to hold the MODVFD register settings  Maybe find a better place for them?
    #ifdef VFD_ENABLE
    Setting_VFD_TYPE = 460, // Select from available VFD types
    Setting_VFD_RPM_HZ = 461, // Set RPM/Hz (not used by all VFD types)
    Setting_VFD_PLUGIN_10 = 462,
    Setting_VFD_PLUGIN_11 = 463,
    Setting_VFD_PLUGIN_12 = 464,
    Setting_VFD_PLUGIN_13 = 465,
    Setting_VFD_PLUGIN_14 = 466,
    Setting_VFD_PLUGIN_15 = 467,
    Setting_VFD_PLUGIN_16 = 468,
    Setting_VFD_PLUGIN_17 = 469,
    Setting_VFD_PLUGIN_18 = 470,
    Setting_VFD_PLUGIN_19 = 471,
    Setting_VFD_PLUGIN_20 = 472,
    Setting_VFD_PLUGIN_21 = 473,         
    #endif

[terjeio]

I still have a few days of hard work in my garden (moving rocks and soil), will look into this and other issues when done.

[andrewmarles]

I would argue that the garden has a higher priority level than any of this VFD stuff. Machines are running and making parts so while the above implementation may not be perfect it seems to at least be useful for people. I could not have done this so easily without the awesome self-documented configuration $ettings management that comes from GRBLHAL+IOSender.

[terjeio]

What is your reasoning for adding the VFD spindle selector setting $490 instead of registering all the spindles with the core and using $395?

Also, my plan was to add a VFD enable number for each type and use that for enabling only one, and use -1 for enabling all. Was that a bad idea?

vfd_spindle.c should be used to register the settings only? And if only one type is enabled only the relevant settings should be made available?
Also VFD_ADDRESS should be made into a setting, possibly at least two considering that I am looking into adding muliti spindle support. Multi spindle support may also require changes to each spindle type allowing multiple instatiations...

[andrewmarles]

What is your reasoning for adding the VFD spindle selector setting $490 instead of registering all the spindles with the core and using $395?

To me this looked like it would limit the number of supported VFDs (potentially dozens) to the number of spindles that can be registered (8) and that didn't seem like it was viable long term as more and more VFDs are added. If there is a way to first select the type of VFD and then register one of only that type, that would be good, but it seemed to me that the spindle registration is happening before the configuration is read from NVS and so I could not get that working, hence the current solution.

Also, my plan was to add a VFD enable number for each type and use that for enabling only one, and use -1 for enabling all. Was that a bad idea?

I think that related to above, most people are only using 1 (or at least < 8) VFD of a specific type, and there are many types of VFD out there. Perhaps this is all less important since it looks like we can likely make the MODVFD mode work for most MODBUS spindles.

vfd_spindle.c should be used to register the settings only? And if only one type is enabled only the relevant settings should be made available? Also VFD_ADDRESS should be made into a setting, possibly at least two considering that I am looking into adding muliti spindle support. Multi spindle support may also require changes to each spindle type allowing multiple instatiations...

Yes, I agree with above. Initially I wanted vfd_spindle.c to just register the settings, but as noted I seemed to hit a problem where GRBLHAL was registering the spindle before it was reading the config, so I addressed that by always registering a "VFD Spindle" and then handling the configuration in real-time after the configuration was read. Perhaps I misunderstood the interactions between the registration and config loading.

[terjeio]

To me this looked like it would limit the number of supported VFDs (potentially dozens) to the number of spindles that can be registered (8) and that didn't seem like it was viable long term as more and more VFDs are added.

That limit is due to me using 3 bits in spindle_settings_flags_t to avoid forcing a settings version update, there are 3 more bits free that can be used but ultimately the spindle type should get its own variable in the spindle_settings_t struct. So long term we are safe.

GRBLHAL was registering the spindle before it was reading the config

Ok, this has to be changed if so. The config function should be called as part of spindle selection after all settings has been loaded.

Initially I wanted vfd_spindle.c to just register the settings, but as noted I seemed to hit a problem where GRBLHAL was registering the spindle before it was reading the config, so I addressed that by always registering a "VFD Spindle" and then handling the configuration in real-time after the configuration was read.

Spindle registration has to be completed before settings are loaded and acted upon. If the spindle type setting is larger than the number of registered spindles it defaults to spindle 0.

Perhaps I misunderstood the interactions between the registration and config loading.

Could be - I'll have to recheck that this is handled in the correct order.

I would suggest that vfd_spindle.c is limited to handling settings only. The vfd_type_t should be replaced with symbol definitions or set to start from 1 to match VFD_ENABLE in my_machine.h? This is the current definition:
//#define VFD_ENABLE 1 // Set to 1 or 2 for Huanyang VFD spindle. More here https://github.com/grblHAL/Plugins_spindle
This should be changed to something like this:
//#define VFD_ENABLE -1 // Set to -1 to include all VFD spindles and to a value > 0 for a specific VFD. More here https://github.com/grblHAL/Plugins_spindle

Thinking aloud, perhaps passing registration through it as well with the registration structure wrapped in super structure could be useful as well? E.g with an optinal function pointer to call on settings changes?

[andrewmarles]

I would suggest that vfd_spindle.c is limited to handling settings only. The vfd_type_t should be replaced with symbol definitions or set to start from 1 to match VFD_ENABLE in my_machine.h? This is the current definition: //#define VFD_ENABLE 1 // Set to 1 or 2 for Huanyang VFD spindle. More here https://github.com/grblHAL/Plugins_spindle This should be changed to something like this: //#define VFD_ENABLE -1 // Set to -1 to include all VFD spindles and to a value > 0 for a specific VFD. More here https://github.com/grblHAL/Plugins_spindle

Thinking aloud, perhaps passing registration through it as well with the registration structure wrapped in super structure could be useful as well? E.g with an optinal function pointer to call on settings changes?

I think that above is basically what I was hoping to achieve. One of the main objectives with my modification was to make it so that a user to could select from either PWM or any supported VFD without having to re-compile the code. So in your above, if we define VFD_ENABLE as -1 at compile time, we will go ahead and register all of the supported VFDs and then the user can select their correct one with $395 instead of $490 and they should have access to all of the supported VFDs and PWM at runtime. Will that have an effect on the usage of M104?

[terjeio]

Will that have an effect on the usage of M104?

It seems so, we have to change M104 to use P for selecting between PWM and the configured ($395) spindle? And make P optional and add another optional parameter (Q?) for selecting a specific spindle regardless of the configured one?

This plays into how to handle multiple spindles by adding support for the $-parameter as well. E.g a lathe with a milling attachment may have two VFD spindles, possibly with the same type of VFD...

[terjeio]

I have reorganized the code a bit and added vfd_spindle.c (as vfd/spindle.c). vfd/spindle.c is stripped down to handle settings and registration with the core only. vfd/spindle.c keep tracks of the VFD spindles, I do not know if this is useful for something.

Adding a new VFD requires a new spindle number in shared.h, and a few lines in vfd/spindle.c similar to how it is done for the modvfd spindle. The new source file(s) should also be added to CMakeLists.txt.

The new settings numbers has been added to the core and $360 changed to the modbus address. $395 is used for configuring the active spindle regardless of spindle type and I have increased the max number of spindles to 32.

I added your modvfd code for basic testing, I can add the other two as well but perhaps you can do it in order to check that my approach makes sense?

[andrewmarles]

Apologies for the delay, but I have been on vacation and consumed with other projects. I have started bringing in the GS20 and YL620 drivers into the plugin, but I have found that with the MODVFD driver that is checked into master in the latest commit d2bc839 I get a hang (board goes unresponsive) whenever I issue a soft-reset when the MODFVD spindle is selected. I also get the hang when I try to switch to a different spindle/vfd. I also get the same thing with the GS20 and YL620 drivers, but since they are so similar to MODVFD that isn't surprising. So far I have been unable to find the cause of this. I have tested on both IMX and STM32F4 drivers and the symptom is the same.

I have updated my vfd_support branch to work with the latest core changes, and I do not get the same hang. I will continue to try to get these merged in.

[terjeio]

No apologies needed...

I get a hang (board goes unresponsive) whenever I issue a soft-reset when the MODFVD spindle is selected.

Try adding this code to modbus_poll() for now:

       case ModBus_Timeout:
            state = ModBus_Silent;
            silence_until = ms + silence_timeout;
            break;

The modvfd register settings should be 16 bit and/or having min and max values specified?
At least there is a "bug" related to this in modvfd_spindleSetState() that has runstop declared as uint8_t...

[terjeio]

I have added your GS20 and YL620 code to the latest commit. Hope this is ok.

[andrewmarles]

Thank you Terje.

[andrewmarles]

I tested the GS20 and MODVFD code in the latest version of the plugin and it worked well. I will go ahead and start using the core version of the plugin in my firmware releases for my boards - it is already part of the flexi-hal releases.

[mundsen]

I have a ZONCN NZ100-2R2G-2 that I was hping to use with my flexi-hal controller
Anyone know if is supported by selecting one of the existing vfd alternatives in iosender? Or should I order a GS20..?
https://www.jbcnc.se/get_file?product_id=952&file_id=397

I have connected it and get response if I turn it on/off from iosender - I`m going to continue testing if I can use it

[andrewmarles]

Use MODVFD to set the appropriate register addresses and values. It works much the same as the Linuxcnc VFDMOD component.

[mundsen]

Ok I'll try that