Board driver using stepper_t variables?

[mfp20]

I noticed HAL drivers use some of the vars in struct stepper_t. Why?

I mean, from my perspective the HAL should use step_outbits and dir_outbits only, as those are the bits needed to set/clear the gpios. Being an HAL driver means it has to provide simple methods for HW access only.

So, it looks like the stepper_t structure isn't really fit for the role; a left over from grbl conversion to HAL'lized grbl? Any chance to change it?

[terjeio]

I noticed HAL drivers use some of the vars in struct stepper_t. Why?

For making spindle synced motion work. And some plugins references fields in stepper_t as well.

So, it looks like the stepper_t structure isn't really fit for the role;

Perhaps not but currently it is, what would an alternative look like?

a left over from grbl conversion to HAL'lized grbl?

Yes, and I have added fields to it and exposed it (making it accessible via the hal.stepper.pulse_start parameter).

Any chance to change it?

Yes, but you will have to move code from the HAL to the core to make spindle synced motion work? And new entries has to be added to the HAL struct for the plugins now hooking into hal.stepper.pulse_start? It will be easier to run the relevant HAL/plugin code on the same core (as stepper.c)?

[terjeio]

I as I said before I believe planner.c, stepper.c and the related interrupt rutines should be all be offloaded to the second core - having stepper data transmitted across cores is IMO only going to slow things down. A clean and lean API between the remaining grblHAL core is what is needed?

Once we have 32bit wide ints and higher cpu freqs, we can easily manage the whole thing in a different way.

grblHAL is not usable on an 8-bit architecture... There is one 16-bit driver (I would be surprised if anyone uses it), the rest is 32-bit. Still, some of the supported processors are memory constrained, both RAM and flash.

using a fixed static memory pool

I plan to malloc at least the planner buffer, perhaps the segment buffer as well - this to make the size run-time configurable. For that linker scripts has to be modified and all the drivers retested... Both are now implemented as linked lists so the code change will be minimal.
I would not attempt introduce high frequency to malloc/free in the code - risking heap fragmentation...

I'm thinking to completely rewrite the motion pipeline for offloaded operations, instead of reusing the same code for both monolithic and offloaded operation.

That is what I wanted/want to do eventually - by having an API that allows planner.c, stepper.c and the related interrupt rutines to have different implementations. As a first step planner.c, stepper.c and the related interrupt rutines can be reused, later on these could be moved to a FPGA, S-curve acceleration implemented and what not. Mach3 has what I belive is such an API roughly at the same place in the code - I briefly looked into it a few years ago. LinuxCNC also has one as there is at least one FPGA implementation - but I have not kooked at that. The Mach3 (and Mach4?) can transfer the needed data via USB or Ethernet (RPC?), dont't know what LinuxCNC does. There is a dev kit for the Mach3 implementation, IIRC with examples. LinuxCNC is open source.

[mfp20]

I as I said before I believe planner.c, stepper.c and the related interrupt rutines should be all be offloaded to the second core - having stepper data transmitted across cores is IMO only going to slow things down. A clean and lean API between the remaining grblHAL core is what is needed?

The clean and lean API is already in place; it's fairly good, the only exception I see is the spindle code being in the board driver instead of the core.

You are referring to the use case 2: that's fairly simple as the irq triggers on the second core (or fpga), ie: the core where all the motion pipeline is computed. Once the small text gcode is handled to the second core, there's no computed motion data passing back to the first core; hal and sys calls only. Once all the calls to hal and sys are atomic (and they mostly are, didn't check them all yet), the job is over.

Other use cases instead - ex: offloading to host - introduce major challenges.

Once we have 32bit wide ints and higher cpu freqs, we can easily manage the whole thing in a different way.

the rest is 32-bit. Still, some of the supported processors are memory constrained, both RAM and flash.

Sure they are, but having 4 bytes wide ints mean you can move 4 steps (8 motor each step) per cycle so the "tx protocol" is much faster. Or 4 chars in other circumstances (ex: 32bit DMA copy). Then there's the frequency: you have much more cycles per second, so you can use fat ISRs and still don't clog the main procedure or having jitter. Having this luxury opens the way to complex code architectures not available on 8bit AVRs... I've been trying for 1 year on those... no way...

In other words we are using a moped engine on a Ferrari frame.

using a fixed static memory pool

I plan to malloc at least the planner buffer, perhaps the segment buffer as well - this to make the size run-time configurable. For that linker scripts has to be modified and all the drivers retested... Both are now implemented as linked lists so the code change will be minimal.
I would not attempt introduce high frequency to malloc/free in the code - risking heap fragmentation...

There's no real need of high freq malloc/free. You malloc once on boot a memory pool for your custom allocator and then manage the pool yourself. The allocator itself has some cpu overhead, but on AVRs it was impossible to think about it; on those faster mcus instead, you can have a simple linear allocator without missing the performance point... and a linear allocator doesn't fragment.

I'm thinking to completely rewrite the motion pipeline for offloaded operations, instead of reusing the same code for both monolithic and offloaded operation.

That is what I wanted/want to do eventually - by having an API that allows planner.c, stepper.c and the related interrupt rutines to have different implementations. As a first step planner.c, stepper.c and the related interrupt rutines can be reused, later on these could be moved to a FPGA, S-curve acceleration implemented and what not.

As I wrote, use case 1 and 2 can be implemented using the grbl pipeline as is. Use cases 3-5 not really, because the data isn't parted in stages. The board driver (ie: last stage in the pipeline) uses data from the planner (ie: stage1)! There's no clean separation. Moreover this data is even copied in some cases; so there's cpu and ram waste. Again: that was meaningful on AVRs...

And still I can't figure out why the board driver needs more than just 2 bits (dir+step) per motor per step.

Mach3 has what I belive is such an API roughly at the same place in the code - I briefly looked into it a few years ago. LinuxCNC also has one as there is at least one FPGA implementation - but I have not kooked at that. The Mach3 (and Mach4?) can transfer the needed data via USB or Ethernet (RPC?), dont't know what LinuxCNC does. There is a dev kit for the Mach3 implementation, IIRC with examples. LinuxCNC is open source.

The code to be run on the capable host isn't really a problem. Once you have an RPi4 or an Intel/AMD cpu, you can use any kind of {cpu & fpu & ram}-intensive algos. I have a python3 re-write of Klipper's code (original one is python2); it has a motion planner for kartesian, corexy... can't remember exactly; I wrote it 2 years ago and then got stuck at the firmware for the mcu. Eventually, a plugin for LinuxCNC can be written in order to teach it how to send grbl-compatible blocks and segments...
Host's code isn't a problem at the moment.

[mfp20]

I as I said before I believe planner.c, stepper.c and the related interrupt rutines should be all be offloaded to the second core

Other use cases instead - ex: offloading to host - introduce major challenges.

Problem solved.

void st_prep_buffer()
{
    // Block step prep buffer, while in a suspend state and there is no suspend motion to execute.
    if (sys.step_control.end_motion)
        return;

    if (BOARD_OFFLOAD_TO_HOST) {

        // TODO pull blocks from host

        // Check if we need to fill the buffer.
        while (segment_buffer_tail != segment_next_head) {
            
            // TODO pull segments from host

            // Segment complete! Increment segment pointers, so stepper ISR can immediately execute it.
            segment_buffer_head = segment_next_head;
            segment_next_head = segment_next_head->next;
        }
    }
    else
    {
         [...existing code...]
    }
}

That's all needed in order to offload to external host. The external host must serialize outgoing st_block_t+segment_t, and mcu deserialize incoming ones. As st_prep_buffer() gets called every time the realtime system have spare time to spend, the buffers will be always full as long as the host is always ready to send. And the host is, as it has more cpu power.

The input part is trivial: a similar approach must be in gc_execute_block(): in the case of offloading to host, it does nothing (ie: doesn't run the planner code, as the blocks and segments are pulled directly in the existing buffers).

status_code_t gc_execute_block(char *block, char *message)
{
    if (BOARD_OFFLOAD_TO_HOST) {
        // do nothing
    }
    else if (BOARD_OFFLOAD_TO_CORE)
    {
        // TODO push to core
    }
    else
    {
         [...existing code...]
    }
}

There might be some 'hal' and 'sys' calls to add but that's the simplest way to implement offloading to host without introducing major changes to data structures. The only thing to test is ... wheter the interface in use (ex: UART, USB) is fast enough to transmit blocks and segments in time.

Gimmie some time and I'll complete the code both for offloading to second core (or fpga), and external host.

[terjeio]

Problem solved.

Nearly? You offloaded far too much in your original approach, this is better. But I think planner.c should be offloaded too, plan_buffer_line() is where the bulk of the traffic across the cores should be? This as st_update_plan_block_parameters() will be called by the planner at a high frequency depending on the gcode.

IMO do not use if...then...else to determine where the data should be sent, use function pointers (or weak function declarations). Function pointers are the most flexible as they can be changed at run time, and if adding those needed to a new struct it will be clear where the crossover traffic takes place.

[mfp20]

Problem solved.

Nearly? You offloaded far too much in your original approach, this is better.

Well, I was trying to offload ALL the math. This one is a compromise instead, because in the interrupt handler there's some math going on as well and it isn't offloaded. Those are just a bunch of sums and shifts, not much, but still... a compromise.
To get ALL the math offloaded there's the need to change data structures and ... we already talked about that.
So at the end of the day, I like this solution A LOT.

But I think planner.c should be offloaded too, plan_buffer_line() is where the bulk of the traffic across the cores should be? This as st_update_plan_block_parameters() will be called by the planner at a high frequency depending on the gcode.

I don't get this part: in the case of BOARD_OFFLOAD_TO_HOST no gcodes should be sent to console, and in the case some gcode arrives there, gc_execute_block() simply does nothing except writing a warning in log/console. There are no calls to motion control, so none to planner as well.

IMO do not use if...then...else to determine where the data should be sent, use function pointers (or weak function declarations). Function pointers are the most flexible as they can be changed at run time, and if adding those needed to a new struct it will be clear where the crossover traffic takes place.

Currently changes are implemented using some defines in config.h, so the offloading is set at compile time to produce an experimental build. But the final version might be able to set offloading at runtime (ex: on boot).
(README.md)