grblHAL / Plugin_plasma

grblHAL plugin for plasma cutters (THC)
Other
13 stars 1 forks source link

Enabling plasma plugin on RP2040 #7

Open JelmerV opened 7 months ago

JelmerV commented 7 months ago

Hi, I am trying to use the PicoCNC board with the RP2040 for a DIY plasma cutter and am trying to get THC working. The analog input will be the ADC on GPIO28, moving the probe pin to 24. I also ordered an I2C ADC chip but that's a later step.

However, I am immediately running into problems while trying to add the plasma plugin. I added an include "plasma.c" within driver.h but it contains redefinitions of multiple functions causing the compilation to fail. (side question: why was the plasma.h file removed in some recent change?) In summary the errors are for redefinitions of the 'digital_out', 'analog_out', 'stepperPulseStart', and 'enumeratePins' functions, and for the 'settings_changed' variable. Here is the full output:

....
[ 17%] Building C object CMakeFiles/grblHAL.dir/ioports_analog.c.obj
/home/jelmer/grblHAL_driver_RP2040/pico_cnc.c:51:13: error: redefinition of 'digital_out'
   51 | static void digital_out (uint8_t port, bool on)
      |             ^~~~~~~~~~~
In file included from /home/jelmer/grblHAL_driver_RP2040/driver.h:207,
                 from /home/jelmer/grblHAL_driver_RP2040/pico_cnc.c:26:
/home/jelmer/grblHAL_driver_RP2040/plasma/thc.c:137:13: note: previous definition of 'digital_out' was here
  137 | static void digital_out (uint8_t portnum, bool on)
      |             ^~~~~~~~~~~
make[2]: *** [CMakeFiles/grblHAL.dir/build.make:202: CMakeFiles/grblHAL.dir/pico_cnc.c.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
/home/jelmer/grblHAL_driver_RP2040/ioports_analog.c:89:13: error: redefinition of 'analog_out'
   89 | static bool analog_out (uint8_t port, float value)
      |             ^~~~~~~~~~
In file included from /home/jelmer/grblHAL_driver_RP2040/driver.h:207,
                 from /home/jelmer/grblHAL_driver_RP2040/ioports_analog.c:22:
/home/jelmer/grblHAL_driver_RP2040/plasma/thc.c:159:13: note: previous definition of 'analog_out' was here
  159 | static bool analog_out (uint8_t portnum, float value)
      |             ^~~~~~~~~~
In file included from /home/jelmer/pico/pico-sdk/src/common/pico_base/include/pico.h:33,
                 from /home/jelmer/pico/pico-sdk/src/common/pico_time/include/pico/time.h:10,
                 from /home/jelmer/grblHAL_driver_RP2040/driver.c:31:
/home/jelmer/grblHAL_driver_RP2040/driver.c:1048:33: error: redefinition of 'stepperPulseStart'
 1048 | static void __not_in_flash_func(stepperPulseStart)(stepper_t *stepper)
      |                                 ^~~~~~~~~~~~~~~~~
/home/jelmer/pico/pico-sdk/src/rp2_common/pico_platform/include/pico/platform.h:265:76: note: in definition of macro '__not_in_flash_func'
  265 | #define __not_in_flash_func(func_name) __not_in_flash(__STRING(func_name)) func_name
      |                                                                            ^~~~~~~~~
In file included from /home/jelmer/grblHAL_driver_RP2040/driver.h:207,
                 from /home/jelmer/grblHAL_driver_RP2040/driver.c:44:
/home/jelmer/grblHAL_driver_RP2040/plasma/thc.c:362:13: note: previous definition of 'stepperPulseStart' was here
  362 | static void stepperPulseStart (stepper_t *stepper)
      |             ^~~~~~~~~~~~~~~~~
/home/jelmer/grblHAL_driver_RP2040/driver.c:1764:6: error: 'settings_changed' redeclared as different kind of symbol
 1764 | void settings_changed (settings_t *settings, settings_changed_flags_t changed)
      |      ^~~~~~~~~~~~~~~~
In file included from /home/jelmer/grblHAL_driver_RP2040/driver.h:207,
                 from /home/jelmer/grblHAL_driver_RP2040/driver.c:44:
/home/jelmer/grblHAL_driver_RP2040/plasma/thc.c:120:29: note: previous declaration of 'settings_changed' was here
  120 | static settings_changed_ptr settings_changed;
      |                             ^~~~~~~~~~~~~~~~
/home/jelmer/grblHAL_driver_RP2040/driver.c:2066:13: error: redefinition of 'enumeratePins'
 2066 | static void enumeratePins (bool low_level, pin_info_ptr pin_info, void *data)
      |             ^~~~~~~~~~~~~
In file included from /home/jelmer/grblHAL_driver_RP2040/driver.h:207,
                 from /home/jelmer/grblHAL_driver_RP2040/driver.c:44:
/home/jelmer/grblHAL_driver_RP2040/plasma/thc.c:656:13: note: previous definition of 'enumeratePins' was here
  656 | static void enumeratePins (bool low_level, pin_info_ptr pin_info, void *data)
      |             ^~~~~~~~~~~~~
make[2]: *** [CMakeFiles/grblHAL.dir/build.make:244: CMakeFiles/grblHAL.dir/ioports_analog.c.obj] Error 1
make[2]: *** [CMakeFiles/grblHAL.dir/build.make:90: CMakeFiles/grblHAL.dir/driver.c.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:1511: CMakeFiles/grblHAL.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

What would be the correct way of activating the plasma plugin? You mention testing this plugin already so i am also curious about the implementation that you use. I can't seem to find a complete example of the implementation within any of the hardware drivers.

Also, should STEP_INJECT_ENABLE be enabled to use THC?

My fork of the RP2040 driver can be found here for reference: https://github.com/JelmerV/grblHAL_driver_RP2040

terjeio commented 7 months ago

You have included the thc.c file in driver.h - this is causing the compilation errors. The correct place is to add it in CMakeLists.txt like the other submodules by including its CMakeLists.txt and also adding it to the libraries section.

I can't seem to find a complete example of the implementation within any of the hardware drivers.

They do not use cmake for building...

Also, should STEP_INJECT_ENABLE be enabled to use THC?

Yes, but you do not need to do it. Adding #define PLASMA_ENABLE 1 to my_machine.h automatically adds STEP_INJECT_ENABLE. Note that I have not tested step injection for the RP2040 - it could be that it is not easy to get right due to using PIO for step generation and is main the reason for why I have not yet added the plugin to this driver.

JelmerV commented 7 months ago

Amazing, thanks for the quick reply! It compiles now so I will do some tests soon.

JelmerV commented 7 months ago

Do you also have advice for adding an analog input? I added the aux analog input to the inputPins definition in driver.c and it shows up when sending $pins along with the inputs claimed by the plasma plugin, however, the analog input does not seem to get a description.

[PIN:10,Aux input 0,P0]
[PIN:11,Aux input 1,Cutter up]
[PIN:12,Aux input 2,Cutter down]
[PIN:9,Aux input 3,Arc ok]
[PIN:28,Aux analog input 0]

Also, when looking at the settings in IO-sender, the plasma settings show up but the $350 options are shown as "off", "N/A", and "Up/Down". Most other settings are also missing. Do you have the reason for that?

My goal is to use the first mode of operation as mentioned in the readme, where $350=0 only an analog voltage signal is needed.

terjeio commented 7 months ago

Code to support analog input has to be added in ioports_analog.c - see the STM32F4xx driver for how it can be done. When added an input can be claimed, $350 will change and the description will be added.

JelmerV commented 7 months ago

Thanks! That is also working now. It took some tinkering but the STM32F4xx as a reference helped a lot.

Sorry for asking these basic questions but I got one more thing. I want to disable the spindle PWM feature. This pin isn't needed for plasma cutting and has an ADC for the analog input, so i want to reuse that. I believe I need to set SPINDLE_ENABLE_PWM0 false to get the basic spindle here in driver_opts.h. Modifying there works, however, I would prefer setting this from the machine definitions in the driver code, without needing to modify the core. Is there a definition that I can to disable spindle PWM and free that pin?

JelmerV commented 7 months ago

Figured it out. Setting #define SPINDLE0_ENABLE SPINDLE_ONOFF0 does the trick.

My version can be found here. I will run some tests once I get the plasma cutter wired up.