[Grbl_Esp32 Issue#689] Huanyang/VFDSpindle communications robustness

#1 – scottbez1 于 2020-12-04

On a related note, I have been digging through the various Huanyang documentation sources to better understand parts of the protocol that are less commonly used, and collecting those notes here: https://paper.dropbox.com/doc/Huanyang-VFD-modbus-protocol–BArwgZ4Tgj_dWv27riC1eFBTAQ-836X8EruAxH21PFe0Z2Um

It’s primarily based on

#2 - terjeio 于 2020-12-04

@scottbez1 Good that you have found a way to get at the actual RPM, this can be used for spindle at speed functionality. Having this will IMO address some of you concerns about robustness.

#3 - scottbez1 于 2020-12-09

Making good progress on this so far - I have a bunch of basic test cases passing, including some that would not pass on the existing implementation (mostly around error handling).

In progress diff against main (1468ff8)

A high level overview of the big architectural changes:

The biggest change is to the VFDSpindle abstraction. The VFD task is no longer a relatively simplistic command sender that pops commands from a queue and sends them with a few retries for good measure, but now actually encodes the expected bidirectional communication pattern with a generic external VFD:
- It continually polls the status of the VFD to make sure communications are healthy and that the VFD is in the desired configuration (rpm+state)
- see read_status
- the subclass implementation of readstatus can send as many commands as necessary to get this information, via the new sendcommand protected helper that VFDSpindle provides (which handles addressing, CRC generation, and response CRC/length validation under the hood)
- If the VFD is not in the desired configuration, it will repeatedly send commands necessary to get it to that state (indefinitely)
- see request_configuration
- again, the subclass implementation of request_configuration can send as many (or as few) commands as necessary

The function comments in VFDSpindle.h describe this updated abstraction in more detail.

A key part of implementing this abstraction is that the modbus "command" queue has been replaced with a "config" queue.
- changes to the requested "config" (rpm/state) are pushed to the queue directly (as a struct of SpindleState and uint32_t rpm), rather than being first converted to a modbus command
- the "config" queue actually has size 1, and config messages are always pushed to the queue using xQueueOverwrite - meaning the latest requested config replaces any previous requests
- if the VFD task were to fall behind due to slow comms or some other reason, we don't actually want to queue and "replay" any intermediate states. It's always best to jump to the current state as soon as possible, so newer requests simply replace older ones
- the queue is really just a threadsafe way to pass the requested rpm/state to the VFD task from the grbl main task; everything necessary for actually executing on that request (deciding whether any communication is necessary, constructing modbus commands, etc) happens in the VFD task and is mediated by the base VFDSpindle, with details (like the actual message bytes) still delegated to the subclass to implement. This means the VFD task can indefinitely retry if needed and will never "drop" anything

So far I've only implemented HuanyangSpindle, but I will look at H2A soon (it's simply commented out in the attached diff to avoid compilation issues since it hasn't been updated yet). It's possible that this new abstraction isn't quite right or generic enough to cleanly support H2A in addition to Huanyang, in which case I'll continue to adapt my approach. I may set this rewrite aside for a little bit now though; I need to finish building my Workbee first so I can actually run full test programs rather than just manually sending simple spindle g-code commands...

#4 - atlaste 于 2020-12-09

I have just scanned your comments and have to look into this in more detail to be honest.

We fully agree that we should err on the side of caution when working with VFD's. In fact, that's the main reason why I firmly believe that RS485 is the correct way to work with VFD's; most other ways to work with them are just one-way communication.

Please work on the Devt branch and not the master branch. That makes a PR and/or merge much easier in the long run, and perhaps some of what you describe is already fixed there.

The reason why there is a queue that's separate from the actual spindle code, is because of the design of grbl. GRBL expects a single, non-blocking thread that basically takes care of motion control. Motion control blocks here also take care of spindle control, and spindle speed is also part of that. Unfortunately, this thread should not be blocked for long time periods of time. In other words, calls to Spindle are expected to be return 'quickly', whatever that may mean.

One of the commands however is the 'mc_dwell' . I have been meaning to change that to accept a function pointer, which can then be used to do a wait operation -say- when something is wrong, or when the spindle is not up to speed. This has everything to do with the safety things you describe, and has been on my TODO list to investigate for quite a while now.

One of the key goals of the VFDSpindle class when I designed it was to make it as much independent as possible of the VFD that's being used. There are many, many, VFD protocols around, and I browsed through a few manuals of Delta, Huanyang (they have different VFD's), my own H2A and some others before I got started. Bottom line, they don't support the same commands at all. Some work with frequencies, others with rpm. Some provide actual rpm feedback (H2A), others just tell you what register values are set (Huanyang apparently). In any event, there is usually some register value you can pull to see if the thing is alive, and normally you get a modbus 'ok' reply when a command is received properly. So, that brings me to the principles you mentioned:

> - the spindle's status (configured- and actual- rpm and state) will be repeatedly read, and an alarm raised in any of these conditions:
> - if we don't receive a response (only if we've previously gotten a response, to avoid raising an alarm immediately if the ESP32 is powered up before the spindle)

I believe this is already the case. The 'unresponsive' flag is supposed to take care of this.

> - if configured state does not match the state we commanded -- this is to prevent motion from continuing to run if the spindle has stopped running from some reason (e.g. if someone pressed the STOP button on the VFD itself, or the VFD turned off the motor due to some fault)

That's a good point. Haven't thought of that, and it's therefore not implemented. Yes I do think this is a good idea if the spindle supports either register value or actual rpm. Most spindles should. As for implementation, see the next point.

> - if configured rpm does not match the rpm we commanded (not sure when or why this would happen, but best to be safe)
> actual rpm is outside a fixed margin of the commanded rpm (except during start/stop) -- I probably won't get to this right away, but seems useful to check

The code in VFDSpindle.cpp (line 78-102) is designed to pull different diagnostics on a regular interval when no commands are being pushed into the queue. This should ensure everything is working as planned. You can simply test the rpm value in the parser, and make it fail if things are not ok.

> - the spindle can't be turned on if the most recent status-read failed -- this is to prevent turning on a spindle that we cannot hear back from, which is risky since we won't know what state it's in

Yes. pollidx == 0 is the special ID that enables initialization of the VFD spindle. For some spindles like the H2A, you need to know some register values before you get started. You can't start a spindle, as long as initialization did not take place.

> - in the event of any spindle error/alarm mentioned above, we will continuously send commands to stop the spindle until one is acknowledged -- this is to prevent a "runaway spindle" so to speak; this way a transient communication error can't cause a "stop spindle" command to be dropped and not re-sent when the connection comes back

I'm not sure if that's a good idea: if you have a spindle speed change that err, it will trigger a spindle stop, which will do damage. That being said, it might be a good idea to mark certain spindle commands as 'required', which means the MAX_ATTEMPTS check will basically never be triggered.

Some of these can be solved if we could somehow make dwells if something is wrong. That would basically just stop the machine, while things are wrong.

I'll drop you some messages on slack, let's see what works best.