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

Proposal: API for secondary inputs in to grblHAL #80

Closed
5ocworkshop opened this issue Oct 31, 2021 · 26 comments
Closed

Proposal: API for secondary inputs in to grblHAL #80

5ocworkshop opened this issue Oct 31, 2021 · 26 comments

Comments

@5ocworkshop
Copy link

Terje,

I'd like to propose the idea of an formal API in to the grblHAL core via an official plugin that defines the functions available, applies restrictions and safety checks needed, and defines the physical interface type(s) for optional connection in to grblHAL.

Serial an i2c are exposed today and can likely cover most applications. Possibly also having USB on the roadmap as an additional option at the hardware layer might be something to consider as well, as that brings both higher bandwidth and also a large variety of pre-existing USB devices to the table.

With so much flexibility and potential in grblHAL, the opportunity for creative development (and use) beyond grbl's core user base and applications is significant and it would help showcase how grblHAL is a generational leap beyond grbl in terms of extensibility.

As an example, the availability of the second serial interface and the ease and elegance of wiring it up to the existing Keypad plugin had me thinking overnight about exploring getting the Xbox One BT controller , working directly via the Pi. I had used this on a previous grbl system via Universal GCode Sender on Windows. The Xbox controller brings better start/stop feedback from the sticks etc. and I believe support on the Pi/Linux side is well established, but it would be great if any work done in this regard was largely re-usable for other types of controllers.

Zooming out, as you demonstrated with your MPG work, a secondary interface could support up to (and beyond?) the capability of a full sender by handshaking with the core and determining that it is safe to take over authority from any existing sender on the primary interface. I was discussing it a bit with Drew and he made the important observation that "There has to be a priority and arbitration." between the two.

Also, by making both the (optional) hardware and software interfaces more formal, board designers can choose to include them or not in their designs, but they can be promoted and plugin work (on both the grblHAL and remote device side) can more easily be shared and improved by a wider audience, regardless of their grblHAL driver.

I am happy to help contribute to the effort if you think it has merit.

@terjeio
Copy link
Contributor

terjeio commented Oct 31, 2021

Serial an i2c are exposed today and can likely cover most applications.

Do not forget about networked connections ;-) I did not bring this into our previous discussion, but there is an event that can be subscribed to that allows seamless switching between streams.

I was discussing it a bit with Drew and he made the important observation that "There has to be a priority and arbitration." between the two.

That is what the MPG mode switching input tries to achieve...

I am happy to help contribute to the effort if you think it has merit.

It sure has - hardware interfaces should be fairly easy to describe, software interfaces not neccesarily so depending on where in a stack it sits. I am here to support you but will not take the lead myself - I, and surely the community, will be pleased if you decide to go ahead.

@terjeio
Copy link
Contributor

terjeio commented Nov 1, 2021

Some drivers implements the opional hal.stream_select API call, one is the iMXRT1062 driver. This is used by stream implementations to take over full control, e.g. by a telnet stream on connect.

The core has basic support for handling the MPG protocol (for a specified stream taking over control via pin signalling). This code is not used by the core but offered for drivers/plugins to use.

Are these a good start for what you envisage? Perhaps improving the interaction between these will be a good start. E.g. a hal.stream_select implementation should deny switching if a MPG stream is in control?

@andrewmarles
Copy link

Yes, the hal.stream_select API is something that has been in my mind for this since I had seen it in the IMX driver. One thing that we are discussing is how to enable stream selection without the use of external pins via some kind of semaphore structure. This could be useful for example if you had a smart pendant connected via bluetooth or UART and wanted to send a macro from the pendant but you did not have a dedicated MPG mode pin.

It may be that the core functionality is already in place and all that we need to do is implement it in the driver, though it would be nice if we could somehow come up with a more general approach that can easily be used across all of the drivers.

@terjeio
Copy link
Contributor

terjeio commented Nov 1, 2021

Out of order commands can be sent via grbl.enqueue_gcode, these will only be accepted if the controller state is Idle, Jog or Tool change. Currently these are not buffered.

Another option is to buffer the characters from a secondary stream until an end of line character is received then temporaily switch the active input stream to read from this buffer. Care must be taken with this approach as one risk intermingling input from the active stream.
Another option that might be safer is to send the input directly to be parsed either as gcode or system commands.

Adding such functionality to the drivers is IMO not a good idea, it will be simpler to add such functionality as plugins - a lot easier to maintain and more flexible.

BTW switching MPG mode on/off does not require a pin, it can be done with by single character commands (top bit set characters?) or whatever else is appropriate.

@5ocworkshop
Copy link
Author

Apologies for the delay. I ran afoul of some issues when I moved external side of my project from a regular Pi to the Pi Zero and that resulted in me being in USB driver jail for a few days. Happily, that is resolved and I'm emerging with a good understanding of how to leverage the kernel event driver for various input devices. I've switched my project to use this now and have it communicating nicely with grblHAL using the keypad module. I hope to clean it up today and implement a config file so it can easily be adjusted to support other types of input. After that is done I hope to test that theory by experimenting with the XBox One Bluetooth controller. I'm optimistic this might come together quickly.

As I work on this I'll continue to think about considerations for the interface and discuss thoughts here.

@5ocworkshop
Copy link
Author

5ocworkshop commented Nov 3, 2021

My present thinking is that I may build on the keypad module under a new name and as I build out the capabilities on the external linux side in the python code, I will update this new module. I think one module on the Linux side will be able to be configured to support many different types of inputs like IR remotes, game controllers, keypads and possibly touchscreens.

I've only been using the my_plugin.c method, so to do this keypad testing I have temporarily removed my RGB light plugin. I am about to add it back but would rather not necessarily merge the two functions. Is there an established way to have two or more "local" plugins? Or, if not, how can I formalize the RGB plugin and move it under it's permanent name?

@terjeio
Copy link
Contributor

terjeio commented Nov 4, 2021

Is there an established way to have two or more "local" plugins?

No, but I can add more. It is easy to add new plugins and make them available to all drivers by adding them to this file.

Or, if not, how can I formalize the RGB plugin and move it under it's permanent name?

This is a tricky one for me - I am a bit hesitant to open the floodgates and accept new plugins (and drivers as well) since that could mean that I have to take over support. A solution would be to accept changes to the file linked above and add links to the external repositories in the plugin repo readme. Users will have to download code from the provided links and add a <plugin>_ENABLE symbols in my_machine.h or on the compile command line to enable them.
A side-effect of doing it like this is that plugins "formalized" this way can have many implementations...

What do you think?

@andrewmarles
Copy link

I do think that there should be a way to formally add plugins to the project (plugins_init.h seems a reasonable place) along with external references in the readme. I think this is somewhat analogous to how boards are added to the various drivers. By keeping the repos external and forcing the user to add the submodule manually I would hope that would draw a line and shift the support burden out to the plugin developer and not on GRBLHAL.

@5ocworkshop
Copy link
Author

I agree with Drew. I think the plugins_init.h and a URL to a readme or repo could point the user from there and draw a key distinction that support and feedback for those plugins is apropriately with the plugin developer.

I'm not sure how this maps with enabling/listing plugins in the my_machine or similar files during initial setup. Perhaps a note needs to be added there to indicate that there are additional, third party plugins, that are detailed in plugin_init.h.

As it relates to the keypad plugin that I'm currently using on the grblhal side for the IR and joystick work, I think I should probably fork it and rename it as serialapi or something similar and maintain my own version of it. It is possible my initial changes would be fine merged in to keypad.c but it is likely better that you aren't burdened with having to review/accept them in future.

@terjeio
Copy link
Contributor

terjeio commented Nov 7, 2021

I'm not sure how this maps with enabling/listing plugins in the my_machine or similar files during initial setup. Perhaps a note needs to be added there to indicate that there are additional, third party plugins, that are detailed in plugin_init.h.

Adding info in my_machine.h is a no-go, all drivers has to be kept in sync with that. It is better to add a link to information in the plugins repo as I suggested above.

I'll try to outline how requests for plugin names (for the init function and the enable symbol) could be handled. Same for setting numbers and M-codes that third-party plugins might need to "go public".

@5ocworkshop
Copy link
Author

5ocworkshop commented Nov 7, 2021

Adding info in my_machine.h is a no-go, all drivers has to be kept in sync with that. It is better to add a link to information in the plugins repo as I suggested above.

I think we're saying the same thing. I wasn't suggesting we add them there, just a single line to alert them that there may be third party plugins available for their platform, and if so any details about them would be over in plugins_init.h. Otherwise, a user wouldn't necessarily expect that there are additional plugin options.

@5ocworkshop
Copy link
Author

I've largely cleaned up and reorganized by RGB plugin so I can make it available as a 1.0 release.

The URL for it is here: https://github.com/5ocworkshop/grblhal-rgb-plugin/tree/main/rgb

If you could add it to the plugins_init.h file that would be great.

I hope to have the serial input/IR remote stuff ready to go in the next day or so as well. I had never appreciated how much time goes in to organizing and annotating all these files. Wow.

@terjeio
Copy link
Contributor

terjeio commented Nov 9, 2021

I have added some info on how to add a plugin to grblHAL.

@5ocworkshop I can add it but I am not so sure I like the name, rgb is IMO a bit non-descriptive. status light or status lights is better?

#if STATUS_LIGHT_ENABLE
    extern void status_light_init (void);
    status_light_init();
#endif

Edit: you may still call the repo rgb-plugin. I have another user wanting to add a plugin for something like these, the same plugin name could be used for that?

@5ocworkshop
Copy link
Author

5ocworkshop commented Nov 10, 2021

Yeah I was torn about the name. I'm not really sensitive about it or the name of the repo. I think it makes practical sense to align them if we're changing the name.

Let me think on it a bit and see if I have any further thoughts.

It should be easy to adapt the plugin for a stack light or to extend it for a Neopixel. The colour setting/light control are functions and definitions abstracted from the actual logic. In the end it is simply a set of rules that trigger aux-out lines. There are likely a few places things should still be abstracted to configs/definition, and I still need to understand how to expose settings via the $ commands.

Drew has requested that I make the R/G/B to aux line mapping a setting you can do from the machine, so you can just wire it up and correct that mapping based on what actually happens. He correctly observed that RGB light strips tend to vary which signal is on which physical line so that would be good.

I'd also like to expose something in the ? status line that tells you what mapped RGB pins are triggered, like you see with the button/probe input, just for troubleshooting.

I think I've talked myself in to the name writing this out. My only change would be I think we should make it plural and use STATUS_LIGHTS_ENABLE. The support for 8 colours today (and potentially full Neopixel spectrum in future) is better represented by the plural.

I just received my new spindle and VFD yesterday and got them spinning on the bench last night, but I have more to do there. So it will be a few days before I can make the changes and re-do the repo but I will put this on my near term list. I also need to mill an insert ring for my bracket for the new spindle.

I thought I had the serialapi / IR Remote stuff finished on the weekend but in final testing I found a small bug where the setting of the ALARM flag is getting cleared somewhere. It wasn't happening before I "cleaned up" my code and moved it around to make it a standalone plugin, so I think it is something I did. Once I find a solution, then I'll be able to post that as well.

I've called that one serialapi for now. I'm not thrilled with the name due to the use of serial for general grbl communication anyway. Perhaps we should invert it and call it apiserial? If you have any thoughts please let me know.

@5ocworkshop
Copy link
Author

One last thought - I was tempted to remove the MCode for turning the inspection (white) light on from the sender, because I added both a physical button control and control form the IR remote. But upon reflection I realized not everyone would have one or both of those, and that i originally just turned it on manually with the code.

I'm not sure an MCode is the right way to do it though, it really isn't a machining related function. Once we figure out $ settings it should probably just be an on/off $ setting and then we could expose it in the IOSender UI potentially, if present in the firmware?

@terjeio
Copy link
Contributor

terjeio commented Nov 10, 2021

Drew has requested that I make the R/G/B to aux line mapping a setting you can do from the machine, so you can just wire it up and correct that mapping based on what actually happens.

This will require changes to the core - or the plugin has to be made driver specific. I have longer term plans to make pin mappings available at run-time, at least for some pins. IMO not an easy task...
Currently it is only possible to assign specific pins to a plugin by modifying the aux pin mappings on a case-by-case basis (by the end user).

My only change would be I think we should make it plural and use STATUS_LIGHTS_ENABLE.

Ok, I can change that in the next commit - unlikely that anybody has started to use the singular version by then.

Perhaps we should invert it and call it apiserial? If you have any thoughts please let me know.

"serialapi" is another name I do not like - "API" is something I write code against, your plugin is perhaps better described as some kind of serial protocol? Your plugin is a layer above the HAL stream API, my guess is that many would regard stream as synonymous with serial thus confusing them? Naming is sometimes a big headache for me...
BTW I want to add functionality for plugins to enumerate stream interfaces and claim exclusive ownership from the init function. Again, I will run into the first come first serve issue that we have with the aux pins - but hopefully that can be solved.

I'm not sure an MCode is the right way to do it though, it really isn't a machining related function.

I have no problem with using M-codes, especially if similar to ones implemented by Marlin. But if it is unlikely they will be used as part of G-code programs then $-commands are to be preferred.

Once we figure out $ settings it should probably just be an on/off $ setting

You mean a $-command or a $-setting stored in non-volatile memory?
Both are possible to add from a plugin, properly defined settings will show up in the ioSender settings UI and commands in the $help commands output.

@andrewmarles
Copy link

andrewmarles commented Nov 10, 2021

I don't think that the it is the MCU pin assignment that need to change, rather it is just that within the plugin the pin assignment can vary. So IOPORTS 0-2 can be taken by the plugin, and then the assignment of 0 - Red, 1 - Green, 2 - Blue can be re-assigned at runtime as per lines 1012 to 1014 of rgb.c. The index constants here could be replaced with $variables that are stored in NVS. I don't think this requires core or driver modification?

@terjeio
Copy link
Contributor

terjeio commented Nov 10, 2021

The index constants here could be replaced with $variables that are stored in NVS. I don't think this requires core or driver modification?

It does not but it leaves the pins accessible from M62-M65, that could be a problem? Currently pin info is stored in an indexed list and the index is used as the pin id - this has to be changed if M62-M65 access is to be blocked for pins claimed by plugins.

@5ocworkshop
Copy link
Author

Naming is sometimes a big headache for me...

Yes, especially so in a multi-platform, modular project like this.

I have no emotional attachment to the name and I had similar struggles when I even chose it. Where I was coming from was that this would be the interface against which external applications that are not streamers (although they could be, we haven't fully defined that yet) can talk to the core, ancillary to the principal interface that already exists via USB or Ethernet.

I was likely thinking of it too narrowly as my use case to date has consisted of the external python script that runs on the Pi Zero that is physically sitting on the 40 pin GPIO header on the HAL2000 board. So far these use cases are secondary or tertiary - like using the IR remote (and hopefully other input devices) to jog the machine, turn on the inspection light etc. I could also envision someone making a consolidated control panel, connected to the Pi, like this one that incorporates various other shop functions that relate to but do not need to be controlled by grblHAL itself: https://forum.onefinitycnc.com/t/information-to-build-you-own-onefinity-companion-control-box/2647

From the Pi development perspective, the presence of the module in grblHAL does expose an interface for someone to write against and we are hoping to standardize that. But even the use of the world serial in the name is suboptimal as it may well be USB or ethernet on a different platform. Perhaps accessory interface or secondary interface or something along those lines would be more appropriate?

@terjeio
Copy link
Contributor

terjeio commented Nov 11, 2021

Perhaps accessory interface or secondary interface or something along those lines would be more appropriate?

"Accessory interface" is better than "secondary interface" as there are many internal interfaces in grblHAL so "secondary" is IMO ambiguous - at least it will be for a plugin programmer?

The pin mapping issue can be solved by adding a remap function and related $-settings for the predefined aux pins that just moves their definitions in the array. This has to be done before a plugin claims any pins. And it may solve a problem I currently have with input pins, not all are interrupt capable and plugins that needs that capability has to claim pins until it finds those it needs that are. On top of this is there is the issue of board labelling...

@karoria
Copy link

karoria commented Nov 11, 2021

I wanted the onboard led of teensy4.1 to show status by blink patterns. I know I can use this suggested module/library with some modifications, but I was thinking a direct coding somewhere as it is a matter of high and low only. Which place is better to add that code? and how to link it?

@terjeio
Copy link
Contributor

terjeio commented Nov 11, 2021

@karoria If for your own use only you can add it by having a my_plugin_init() function in the code, this will be called automagically at startup and is where it "hooks" into the system by modifying function pointers. Here is a "blinky" example for STM32F411 that initializes the LED connected pin. The downside to this method is that such a plugin is bound to a specific MCU and board...

The file containing the code should be placed in the same directory as driver.c and is linked automatically.

@karoria
Copy link

karoria commented Nov 11, 2021

Thanks a lot Terje! Yes, it is for my personal use only.

@terjeio
Copy link
Contributor

terjeio commented Nov 11, 2021

I have added a discussion related to claiming aux ports in plugins. Add to that if you have comments.

@5ocworkshop
Copy link
Author

Accessory interface" is better than "secondary interface"

Ok, agreed. I'll adjust things so it refers to the concept of an accessory interface and call the module acc_if just for convenience.

@terjeio
Copy link
Contributor

terjeio commented Nov 28, 2021

For info: API for claiming serial port(s) in plugins has been added in the latest release. An example.

@terjeio terjeio closed this as completed Apr 16, 2024
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

4 participants