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

Enabling plasma plugin on RP2040 #7

Open
JelmerV opened this issue Feb 9, 2024 · 6 comments
Open

Enabling plasma plugin on RP2040 #7

JelmerV opened this issue Feb 9, 2024 · 6 comments

Comments

@JelmerV
Copy link

JelmerV commented Feb 9, 2024

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
Copy link
Contributor

terjeio commented Feb 9, 2024

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
Copy link
Author

JelmerV commented Feb 9, 2024

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

@JelmerV
Copy link
Author

JelmerV commented Feb 10, 2024

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
Copy link
Contributor

terjeio commented Feb 10, 2024

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
Copy link
Author

JelmerV commented Feb 11, 2024

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
Copy link
Author

JelmerV commented Feb 11, 2024

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.

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

No branches or pull requests

2 participants