Open Moffy opened 4 years ago
Are you on Windows? You should set up the environment as I suggested (but for MCUXpresso), or commiting changes would become cumbersome.
Do not modify the LPC1769 driver in situ, create a new driver folder, IMXRT1020
, and copy the files over. But then maybe you did that already :)
I am on Skype and with the same username as my github repository - if needed I can connect with you via Skype as well. Sometimes it is better to talk...
When I write I new driver I disable all hardware configs/access in driver.c and focus on getting communication up and running. After that I start with the simple I/O (coolant, spindle, step and dir outputs), then getting the stepper/pulse timers and generation up and running and finally spindle PWM. The hard part after this is often making the driver flexible for different pin mappings and the like. If your only target is only going to be the RT1020 devboard you can avoid a lot of headaches...
After updating my MCUXpresso I will add the RT1020 and take a look at the CMSIS files to be able better guide you.
I have been giving the project some thought, and I will need to stick with the MCUExpresso and the NXP SDK as the basis. It is what I know, and the SDK makes it easy to add hardware features such as SD, Ethernet, SDRAM etc and they are all consistant with one another, so less coding required. I was looking at your HAL, very neat, a list of function pointers that can easily be substituted for something else: e.g. UART for USB etc. and as you mentioned you could do this on the fly. But of course flexibility comes at the cost of complexity. Undecided about the trade off, as I do not have anything like your proficiency. But I found a good place to start, with communications as you suggested, and the NXP SDK has this lovely thing called the Serial Manager that can switch between SWO, UART, and USB/Virtual USB Serial port. I'll look into it further.
I have been giving the project some thought, and I will need to stick with the MCUExpresso and the NXP SDK as the basis. It is what I know, and the SDK makes it easy to add hardware features such as SD, Ethernet, SDRAM etc and they are all consistant with one another, so less coding required.
MCUXpresso should be used indeed - I do as well. But please do use the approach described for Arduino development to make integration with github streamlined.
I am downloading the board SDK for the 1020 now, and I have included SD card and Ethernet support in that. Higher level grblHAL support for SD card and Ethernet is on top of FatFS and lwIP respectively, and luckily that is what the SDK provides. This makes it even easier to add this functionality to your driver.
The advice you offered about setting up the environment does not seem to apply to the RT1020. You use the teensy 4.x/Arduino SDK to compile against, not real familiar with that. Maybe one of the teensy 3.x's are RT1020 based, but again not familiar with that. I realise that since NXP moved from LPCOpen to MCUXpresso the project files seem to have multiplied, but when it comes to writing the driver it's a matter of providing an equivalent set of functions and the SDK is secondary(except for me I am more familiar with the NXP SDK). Also your driver based on the LPC1769 compiles just fine with all the MCUXpresso tools. I feel I am missing something basic in your explanation, sorry. As a starting point I have tried the "Hello World" project with cdc_vcom. The debug terminal allows for UART, vcom serial and USB switching in the header. It provides all the buffering , blocking, non blocking etc. Seems a solid starting point, unless I am not understanding your point. Thanks
The advice you offered about setting up the environment does not seem to apply to the RT1020. You use the teensy 4.x/Arduino SDK to compile against, not real familiar with that.
It has nothing to do with whick SDK or libraries to use and everything to to with avoiding copying files when syncing with the repository.
To get the LPC1769 driver to compile you had to at least copy the contents of the grbl folder to the driver folder. If you did this in the cloned directory structure and imported the LPC1769 project from there then it will compile just fine. However, if you now want to commit any changes back to the repository the files in the grbl folder has to be ignored to not upset the repository organization (the grbl folder in the driver folder does not contain any files and should not).
You can of course handle this manually or by adding .gitignore filters, but there is a better way as described for the Arduino. The simple idea is to keep the cloned directory "as is" and not copy files around complicating commits. To achieve this in an elegant way symbolic links comes to the rescue. This works by creating a virtual directory structure that contains the files needed for the project linked to the original clone. Any changes to files and folder contents in the project folder is then actually stored in the clone. This keeps the clone structure intact, and it is now straightforward to commit changes or even update the clone from the master. The symbolic links keeps files and folders in sync.
As a starting point I have tried the "Hello World" project with cdc_vcom. The debug terminal allows for UART, vcom serial and USB switching in the header. It provides all the buffering , blocking, non blocking etc. Seems a solid starting point, unless I am not understanding your point. Thanks
This works fine but not so with grbl out of the box. This since grbl handles some characters in a special way, so called real time commands. They are picked off the incoming data stream as they are received and sent to a handler function in protocol.c, literally jumping the queue. Support for the grblHAL tool change protocol also requires special handling, I have implemented this by having two input buffers that can be switched in and out dynamically. It is possible to add another buffer in between the one the SDK provides and the grblHAL core, but then this is will become an extra layer of complexity and overhead.
Adding input handling from a SD card or ethernet will require additional code in the secondary buffer code too. By using function pointers to access the functions that read and write data to the I/O stream grblHAL avoids this extra layer and the complexity that goes with it. It also has allowed me to provide plugins for handling SD card streaming and network protocols, making it a lot easier to add this functionality to a driver. BTW the SD card plugin relies on processing of real time commands from the "normal" input stream. This to allow feed hold, job termination etc.
So yes, the HAL adds a bit of complexity - but it also adds a lot of flexibility.
Glad that the NXP SDK isn't the issue. I will have to research the virtual directories, I have a vague idea of what you are describing but will need to research fuller to understand the detail. But it sounds like a good idea. I ran across the problem with the "realtime codes" and noticed your use of two functions, one pointer which was inserted into the USB endpoint handler, and the serial ISR respectively. The SerialManager does seem to bury these routines under several layers, so yeah there is a real issue there. I'll think some more about it and research the "virtual directories" as well so that I have the nuts and bolts down.
So symbolic links allow the static code(unchanging) to be linked with the dynamic code(driver, changing) by means of a symbolic directory. Is this a github thing or an OS thing or both? By the way I am using Windows.
I have made a rough start on the serial comms. Find the attached file. Your advice is appreciated. Serial.zip
So symbolic links allow the static code(unchanging) to be linked with the dynamic code(driver, changing) by means of a symbolic directory. Is this a github thing or an OS thing or both? By the way I am using Windows.
It is an OS thing. Windows is a bit late with this and there is no good UI support from Microsoft - the Link Shell Extesion fixes that. Unix/Linux has had support for symbolic links in the filesystem since way back before I started coding.
I have made a rough start on the serial comms. Find the attached file. Your advice is appreciated.
If you use the NXP tools to configure peripherals then you should not copy code from the libraries like you have done in serial.c. The calls to initialize the MCU and the peripherals should already be present in the main project file (containing the main function).
You have mixed USB and UART code in serial.c, I prefer to keep USB and UART code in separate files, the LPC1769 driver I have usb_serial.c and serial.c respectively. You should focus on bringing only one of them up as a start.
If you want to start with the UART you will need an external UART <-> USB breakout board AFAIKT. This has to be connected to the Arduino connector it seems. You have already done this? The UART code for the iMXRT1060 driver can be used, perhaps with little or no modifications as it looks like the UART peripheral is the same in both processors. Note that I have named the file uart.c in this driver due to a name clash with the Arduino framework.
The USB code simpler in a way, as you should be able too hook in to the USB driver library as I have done for all drivers that supports native USB communication. Which driver that could be the best starting point is not easy for me to tell without digging into code. If you have a simple USB example project it would be easier for me to find the correct library file.
Thanks for the link to "Link Shell Extension", I found it yesterday but wasn't sure if it was what I was looking for. Not sure about the need to separate the code into UART/USB as the debugconsole combines them into a single unit and that is what is being configured. Also some of the board code needs to be there because the IDE configures some parts uniquely i.e. IOMUXC, but also in this case the MPU. Some of that stuff should probably appear in my own board.c. I don't know if you noticed but the receive routine passes through a regular SysInt interrupt. That is to allow the real time commands to be picked off first. As mentioned previously, the ISR's for serial/usb comms are buried by their own HAL using debugconsole/serialmanager. Thanks for the advice for the serial to usb breakout, I am a qualified Electronics Engineer, so hardware is kinda my thing, but software, not so much.:) There is already a uart serial comms that connects via LPUART1 to the usb debug port on the board and a PC. Also the USB OTG port can be configured as per the code I uploaded for serial comms using debugconsole. Both work quite happily in the SDK examples: "Hello World" and "Hello World virtual com". How do I compile the code to test just the serial communications with the grblHAL code? You have been through it numerous times. Thanks.
I do not know what the debug console is and it may complicate matters? Generally I do not like frameworks as the are often bloated and inefficient. What they are good for is initialization of peripherals. I do use framework code for complicated stuff such USB and networking but thats about it. GPIO and UART handling I prefer to do myself at the register level for speed and simplicity.
As an example I have managed to create a stripped down driver that compiles but fails to link. It relies on the framework for initialization of the UART but I use the code from the iMXRT1060 driver for handling the I/O. Basically the iMXRT1060 serial.c could be used "as-is" since the UART peripherals has the same register structure. I removed the initialization code as this is handled by the board init and fixed a bunch of symbols to match the 1020 definitions. I had to modify startup_mimxrt1021.c to "sneak in" the UART interrupt handler, this by including serial.h. With luck this driver will greet you with the grbl startup message and respond to commands. If so then the first hurdle is overcome.
The link failure is due to the executable beeing too large to be placed in SRAM. How one is expected to handle this I do not know, for you to investigate?
Memory region Used Size Region Size %age Used
SRAM_DTC: 173904 B 64 KB 265.36%
SRAM_ITC: 0 GB 64 KB 0.00%
Thanks I'll have a look at it. I must agree that frameworks are bloated.
I have managed to get it to compile and load and got a little comms before it hard faulted, but using an SDK example for LPUART interrupt. Getting there slowly. I ran into a couple of minor issues: Seems the SDK has a function named "uitoa" so I temporarily renamed the grblHAL to "unitoa". Appears the SDK doesn't have truncf, roundf or lroundf, so created some dummy's temporarily. Will keep working until I get comms. Cheers.
I ran into a couple of minor issues: Seems the SDK has a function named "uitoa" so I temporarily renamed the grblHAL to "unitoa". Appears the SDK doesn't have truncf, roundf or lroundf, so created some dummy's temporarily. Will keep working until I get comms. Cheers.
I changed the library to Newlib in the project above to fix that:
That worked, very nice.
I have decided to take a small step backwards as I haven't really understood how ring buffers worked. They are deceptively simple as I have found out, so I decided to write a set of routines for myself just to make sure I understand them. Then integrate them into a serial lpuart routine. Also the SDK example "lpuart_interrupt_transfer" has a callback function that the programmer(me) writes and gets inserted into the already written ISR. Will give that a go as well.
I have managed to get the serial comms working with the SDK framework. The newest parts are the transmit routines with the callback function and my own ring buffers. It's a bit rough and will need refinement but at least I understand what is going on. The receive callback function does the receive interrupt character processing and transfers the non realtime characters into the ring buffer. Hope you are still there. grblHALserialcomms.zip
Good, then it is time to boot into grblHAL by calling grbl_enter()
in your main()
function. You should then be greeted with the startup message and be able to issue commands.
Note that since you are not using the structs in grbl/stream.h for ring buffer data you have to change hal.rx_buffer_size = RX_BUFFER_SIZE;
in driver.c to the correct size or you may get problems with senders later on.
Yes, I got it to compile and run with "grbl_enter()" had to disable the NVS(Non Volatile Storage?) which caused a fault. But I did get a $ list of parameters. Not accepting input text yet, but I do need to configure I/O pins to suit the board as that hasn't been done yet. Will also try to clean up my serial code to avoid duplication.
Comms is working, just have a small problem with some data always left in the Tx buffer, I'll fix that. Have included my own header for the Arduino pins, that seems ok too. What would you suggest is the best sequence to progress?
What would you suggest is the best sequence to progress?
You may want to move 3 down on the list if you want to see motors moving early.
Note that for 4 the stepper timer clock frequency has to be assigned tohal.f_step_timer
in driver_init()
.
Pin configuration is controlled by settings: pullup/pulldown resistor, interrupt on rising or falling edge etc.
When the basic stuff is working then add serial over native USB, ethernet, plugin support etc. if you feel so inclined.
Excellent ! I appreciate your advice. I solved the problem of data left in the transmit ring buffer, but then there was a corruption of the first few bytes(SDK transmit routine was clearing its busy flag a bit early), solved that by adding my own flag. OK, I'll get started on simple outputs and the probe input. Have already set up the inputs with the appropriate pullups and pulldowns, so that they read as not triggered without any connections.
Managed to get a lot written then my board shutdown. Couldn't program it or anything. Programmed one of my LPC-LINK2 boards as a Segger J_LINK debugger and still no success over the jtag port, no connection. So I changed the boot rom switches to serial and bingo the programmer can connect. Will have to see if I can get things back to a working state.
Ouch, if you have not fried the JTAG/SWJ pins could it be that you have accidentally disabled the interfaces? I have not studied the manual to see if this is possible, but I know I can do that for the STM32 processors I have worked with.
Good luck!
Something like that. It's working again and I think that It's because one of the Arduino interface pins (using it for Z stepper) is shared with JTAG_RSTn. Why would you design a board like that? Anyway after stopping it booting from flash the pin became free for JTAG/SWD again. Loaded "Blinky" into it and the LED is flashing nicely.:)
It's been a little while but I had a few persistent issues with the NXP framework and getting another version of USB VCOM working with grblHAL. Looks like it is working now, just like the LPUART version, it gains the extra 2 pins as well. Now that I am comfortable with the ring buffer I have removed ringbuffer.c/h from the code and reverted to using your stream types for tx and rx. The serial receive code is identical for LPUART and USB except for the small amount in the callback functions. All the transmit and receive fits within the SDK callback functions and works pretty similarly to the original comms code of GRBL except the callback functions replace the ISRs. No need for any SysInt type hacks. If you are interested I have uploaded the code. Note: It is currently setup for USB VCOM, I haven't yet set up easy switching between that and the LPUART. There are a number of Preprocessor directives that need changing in the properties dialog, but that's for another day. grblHALusb_serialcommsv0.3.zip
Yeah, I now have stepper activity!
Great! It is great fun when things start to move.
It's mostly done, at least coding. Will have to test all the functionality. grblHALusb_serialcommsv0.5.zip
Latest code. Tested the PWM code and it worked first time. Starting the ZLevel/autolevelling code. I would like to thank Terjeio for his help, I am not sure I would have succeded without it. Thanks. grblHALusb_serialcommsv0.55.zip
I have taken a quick look at your code and have a couple of suggestions:
Please use this format for the build date to conform with other drivers and so that senders can parse it:
hal.driver_version = "YYMMDD"; // Set to build date
so "2020_11_28"
should be changed to "201128"
I have left out the century on purpose to keep it distinct from the grblHAL build date.
Statements like this can be simplified
GPIO_PinWrite(STEP_PORT, Y_DIRECTION_PIN, ((dir_outbits.value &0x02)>>1));
to
GPIO_PinWrite(STEP_PORT, Y_DIRECTION_PIN, dir_outbits.y);
since axes_signals_t
is a union. IMO easier to read, safer and perhaps faster too as ARM has dedicated bitfield instructions.
and
signals.value = signals.value | (((port >> FEED_HOLD_PIN) & 0x01) << 1);
to
signals.feed_hold = (port >> FEED_HOLD_PIN) & 0x01;
etc...
I always decorate float constants with f
, e.g. 1.0f
instead of 1.0
, to avoid double calculations and casting from/to float. Perhaps not always needed but a good habit?
Excellent, I'll get those things changed, simpler and more readable.
Added user_mcode.check, user_mcode.validate as well as user_mcode_sync. grblHALusb_serialcommsv0.61.zip
Moved the user_mcode_sync from execute to validate. grblHALusb_serialcommsv0.62.zip
Added a new M code for a debug report. Put a condition on the stepper ISR to hopefully stop tracking if a Z movement is active. grblHALusb_serialcommsv0.63.zip
I see that you have modified stepper.c, IMO this is not needed - it is likely you can do the processing in driver.c, or even in ZLevel.c if you redirect hal.stepper.pulse_start
. stepper.c is in the core and should not access anything externally directly - only via the HAL. Yesterday I added a 32-bit DAC for outputting X and Y as analog signals via a board specific file, here is how I did that:
Variables to hold the original hal pointers is needed:
static settings_changed_ptr settings_changed;
static settings_changed_ptr settings_changed;
The code executed in addition to the original code, it just saves the scaled x and y positions in an array for DMA:
static void stepperPulseStart (stepper_t *stepper)
{
count[0] = sys_position[0] << 14;
count[1] = sys_position[1] << 14;
stepper_pulse_start(stepper); // call the original code
}
Since hal.stepper.pulse_start
might be changed on a settings change I had to check for that.
// Reclaim entry points that may have been changed on settings change.
static void onSettingsChanged (settings_t *settings)
{
settings_changed(settings);
if(hal.stepper.pulse_start != stepperPulseStart) {
stepper_pulse_start = hal.stepper.pulse_start;
hal.stepper.pulse_start = stepperPulseStart;
}
}
Note that the stepper argument
has pointers to some of the internal data structures used in stepper.c.
In the init function I added this to redirect entry points:
void int_fn ()
{
...
settings_changed = hal.settings_changed;
hal.settings_changed = onSettingsChanged;
stepper_pulse_start = hal.stepper.pulse_start;
hal.stepper.pulse_start = stepperPulseStart;
}
I you can do it like this neither the core nor the driver (except for the init call) needs to know about the autolevelling code.
I see that you are changing sys_position - this may backfire as it could lead to parser and planner positions getting out of sync. I am not entirely sure about this, needs to be checked.
Ok, I'll give it some thought, but the stepper ISR seems the right place, makes the z steps more obvious rather than abstracting it further. Also using an #ifdef removes it if not needed. Lastly checking for the condition of Z movement to disable the Z tracking is possibly best done there. Changing the sys_position affecting the parser and planner would be a pain, good thought, will have to look into it also. I assumed that since the pulses were counted the real position was updated from there. But that is not what the planner works off because it has to know these things in advance of the movement. Might have to revise how it is done. Thanks.
Ok, I'll give it some thought, but the stepper ISR seems the right place
Code run via hal.stepper.pulse_start
is in the context of the stepper ISR... You can check for any z-axis movement in the current block via stepper->exec_block->steps[Z_AXIS] > 0
.
Thanks.
"stepper_t *stepper" I assume that refers to 'static stepper_t st' as declared in stepper.c. The 'static' makes it not visible outside stepper.c, so am I missing something or does stepper.c need some modification, either removing the 'static' or providing a non static function that returns a pointer to 'st'?
The 'static' makes it not visible outside stepper.c
It is not visible as a global struct, but can be accessed via the pointer argument to hal.stepper.pulse_start
functions. Or do you need global access to it for some reason?
I have searched for 'pulse_start' and 'stepperPulseStart' and can find their definitions, but I cannot find where they are called:e.g. ..pulse_start(&st) or some such. It's driving me crazy, can you help me out.
I have searched for 'pulse_start' and 'stepperPulseStart' and can find their definitions
Their definitions (signature) is grbl/hal.h - you are free to chose whatever name you want for the implementation . You bind the function to the function pointer by assigning it:
From the example above:
hal.stepper.pulse_start = stepperPulseStart;
"binds" stepperPulseStart
to hal.stepper.pulse_start
.
static void stepperPulseStart (stepper_t *stepper)
{
count[0] = sys_position[0] << 14;
count[1] = sys_position[1] << 14;
stepper_pulse_start(stepper); // call the original code
}
calling hal.stepper.pulse_start(&st)
from the core, in this case stepper.c, will then be the same as calling stepperPulseStart(&st)
:
ISR_CODE void stepper_driver_interrupt_handler (void)
{
#ifdef ENABLE_BACKLASH_COMPENSATION
static bool backlash_motion;
#endif
// Start a step pulse when there is a block to execute.
if(st.exec_block) {
hal.stepper.pulse_start(&st);
st.new_block = st.dir_change = false;
...
The beauty of this is that calling via function pointers can be used to completely disconnect the naming of functions implementing the API "contract" from the core and that it allows dynamic assignment of functions. Dynamic assignment means that fucntions can be swapping in or out at run time depending what task is at hand.
For example my drivers typically implements two versions of the hal.stepper.pulse_start
"contract" which is to output step pulses. One is when no pulse delay is called for one is when it is. By switching between them I can skip the extra test for the delay setting in the first and handle it directly in the next. BTW this is done on a settings change and is the reason for why a settings change needs to be "trapped" so that the hal.stepper.pulse_start
can be set to the desired target function in the example above.
Using function pointers also allow function "chaining", I use that a lot to implement plugins as each can add to functionality dynamically. This works by saving away the current pointer before modifying it, and calling the original function via the saved away pointer either before or after our own code.
I think I have a simple solution to the problem of loss of sync between the planner and the machine during Z tracking.
repeat 1 to 5 as needed.
v0.64 Example Code
M100 P0.23 response 'ok' G01 Z-0.2 F50 //offset is 0.23 response 'ok' G01 X,Y motions response 'ok' M101 response 'zoff' wait for it G01 Z5.0 F100 response 'ok'
and repeat grblHALusb_serialcommsv0.64.zip
Resync the planner so that the machine coords are copied to the planner coords using 'plan_sync_position()'.
You should resync the parser too by calling sync_position() instead?
M101 response 'zoff' wait for it
This is asking for trouble as senders has to be made aware of the new response. And it is not compliant with the protocol, either 'ok' or 'error:xx' is to be returned.
So better to delay until XY is complete? You can listen to state changes by attaching a function to grbl.on_state_change
, on M101 wait for STATE_CYCLE to change to STATE_IDLE? Or just call protocol_buffer_synchronize()
? If this is not possible because a handshake with the sender is required it has to be implemented differently.
Thanks for the info about 'sync_position()'. About the 'zoff', its only temporary, for testing purposes and as I mentioned before I will be modifying 'Candle' to handle this response. I first have to get the basic tracking working and tested. About GRBL Senders, I have found only Candle will reliably connect with my RT1021, even though when I use 'Serial Monitor' in the Arduino IDE everything looks fine. By and large they seem very fussy, and since I haven't implemented an EEPROM the error:7 seems to throw them.
Changed M101 response from 'zoff' to 'ok1'. If the 'ok' messages are numbered then they can have distinction, like 'ok1' == 'last command finished executing' etc.
About GRBL Senders, I have found only Candle will reliably connect with my RT1021
Have you tried with setting compatibility level > 0?
By and large they seem very fussy, and since I haven't implemented an EEPROM the error:7 seems to throw them.
This is something no sender expects to see as it is usually only sent on the first boot after a version with additional settings is flashed. Perhaps eeprom emulation code from Teensy 4 can be used as a template for the 1021? Hmm, perhaps grblHAL should not output error:7 when hal.nvs.type == NVS_None?
Changed M101 response from 'zoff' to 'ok1'. If the 'ok' messages are numbered then they can have distinction, like 'ok1' == 'last command finished executing' etc.
I have to admit I do not understand why you need this handshake with the sender. Delaying the "ok" message will not work?
If not then here is an example of how I added handshake it when I needed cooperation from the sender for tool changes (M6). Handshake is needed for that because streaming from the file has be suspended and jogging and some $-commands allowed. This change is likely to be backwards compatible because M6 is not supported by legacy grbl.
Don't really need the 'ok1' just for debugging purposes, the 'ok' comes after the 'ok1' (I tried it) so I can sync off the 'ok' just fine. Yeah, the error:7 doesn't seem like the best way to start comms, and I agree that with NVS_None it shouldn't be sent. It is a feature not a necessity, as long as the '$nn= xxx' generates an appropriate response when there is no NVS. At some stage I will try to emulate the EEPROM with the external FLASH, I'm not sure how applicable the Teensy4 code will be as we are using different SDK's, but there are a few examples in the NXP SDK I can draw from.
Hello I have set up the repo as requested. I have managed to get the LPC1769 driver to compile in MCUExpresso, will have a think about configuring for the RT1020. Might use an example and clean out most code and put things like all the "fsl_..." drivers and CMSIS in their directories and work from there.