MaslowCNC / Firmware

Maslow Firmware
GNU General Public License v3.0
261 stars 136 forks source link

Adds a velocity output to the Report (and others to keep the same # of axes) #376

Closed thormj closed 6 years ago

thormj commented 6 years ago

Corresponding with GC Pull Request #599, Adds another axis to the readout indicating machine feedrate (so GroundControl can pick it up and display it)

krkeegan commented 6 years ago

So, I still think this is helpful, but this only reports what the requested feedrate is, not the actual which could be less due to axis limitations.

blurfl commented 6 years ago

If the requested feed rate is more than the hardcoded limit, which would this show? If it shows the requested rate, maybe it could change color to indicate when that is beyond the limits? Not how the firmware would communicate that fact, maybe a negative value for the response...

In testing, this does appear to report the request.

krkeegan commented 6 years ago

I think in most cases this will be adjusted to the max feed rate. But at least in the case of a coordinated move if the zaxis rate is exceeded, this rate will not reflect the actual rate.

thormj commented 6 years ago

Err...ok. I guess I jumped the gun when I saw the feedrate in the sys structure. I was hoping to return the current feedrate from the velocity controllers up to GroundControl... and then we can decide in GroundControl what to do if the feedrate we get back doesn't match the feedrate that we requested (if the difference is >X for Y seconds and we're supposed to be moving and...).

krkeegan commented 6 years ago

Like I said, I think it is fine for now.

Many things need work. The future changes include better gcode parsing and acceleration ramping. When this happens, I can make sure your routine is handed a proper current feedrate output.

BarbourSmith commented 6 years ago

Very cool! It works for me too

image

This is a great idea to remind us of what feed rates we are actually using.

I have one request which is that the number reported seems a little bit strange when the machine isn't running a file.

Here's what I am seeing while moving the machine around with the arrow buttons before running the file. It was actually moving so the velocity was not zero.

image

Here is what I am seeing when moving the machine around with the arrow keys after running a file. Again, the velocity is not actually 30ipm (that's just what the file was run at)

image

I think the issue is that the G0 command for moving to a new location does not specify a speed, it just goes "as fast as possible" which I believe in our case is hard coded to 1000mm/min

    if (G0orG1 == 1){
        //if this is a regular move
        coordinatedMove(xgoto, ygoto, zgoto, sys.feedrate); //The XY move is performed
    }
    else{
        //if this is a rapid move
        coordinatedMove(xgoto, ygoto, zgoto, 1000); //move the same as a regular move, but go fast
    }

Would it be possible to report a speed of 1000mm/min when doing rapid moves, or is there a reason that would be very difficult to do?

blurfl commented 6 years ago

The right-hand column is generated in frontPage.py from the gcode line sent. Maybe a special case for G0 codes reporting something like 'max.'?... The left-hand column is harder, as you point out. Wouldn't want to mess with sys.feedrate.

thormj commented 6 years ago

I (just) added code to FrontPage.py so that when you're manually moving around, the right-hand column changes to [MAN] (If GroundControl knows when the machine is stopped, I'll put a "0" in there when GC thinks it's stopped, but I haven't found that yet).

@krkeegan said that when they get acceleration working we'll revisit this to indicate actual feedrates instead of requested feeds. I was halfway thinking of mocking it with the PID velocity controllers saying:

realfeedrate = sys.feedrate realfeedrate *= pid_output/pid_input for each attached axis

... but I'm not sure what that would actually look like (and my Maslow is down at the moment due to cleaning...) so I was hesitant to add it yet (did not want to surprise users with really weird numbers).

BarbourSmith commented 6 years ago

What if we computed the velocity on the Ground Control side from the position information being sent from the machine? That way it would always match exactly how fast the machine is really moving.

thormj commented 6 years ago

Now that might be an interesting idea that doesn't "mess with" g-code interoperability (adding "current velocity" = 4th Axis might be considered a hack). OTOH, if your g-code said "make a 358 degree arc", and your update rate was too slow to catch the tool moving, you'd get a very low velocity compared to what it was actually doing.

OTOH, asking the machine for it's velocity would return 0 if it had just finished the move, so "asking the machine" isn't really a panacea for too slow of an update rate, and at 20/sec, we've got a fast enough update rate.

My goal with this was to highlight when Maslow wasn't able to achieve the speed commanded (mainly because I keep forgetting to change the IPM settings in MakerCam)... and maybe store that "correction factor" to be able to make some ETA readouts...

blurfl commented 6 years ago

I think letting ground control do the calculation is a good solution. That way the firmware doesn’t need to be altered, and ground control is keeping track of the location of the bit anyway.

thormj commented 6 years ago

I put in a new patch on that pull request... I'm AFM (away from Maslow) right now so it's untested...

thormj commented 6 years ago

I'm BAM (Back @ Maslow), so i fixed the other pull request. This one can be closed as "not needed."

Is that something I do or something the maintainers do?

thormj commented 6 years ago

GC PR #599 calculates velocity locally and does not need this any more.

davidelang commented 6 years ago

On Wed, 31 Jan 2018, BarbourSmith wrote:

What if we computed the velocity on the Ground Control side from the position information being sent from the machine? That way it would always match exactly how fast the machine is really moving.

doesn't one of the layers of PID loops define the desired velocity?

thormj commented 6 years ago

It does. My old patch to the firmware tried exposed that as a "4th axis" with the readout statements, but as it turns out, sys.feedrate has the last feedrate requested and didn't reflect the actual feedrates. This latest version computes it based on distance moved (so if you're doing very curvy, very fast, the velocity shown will be low since GC didn't take the curve into account when computing the velocity).

davidelang commented 6 years ago

On Thu, 1 Feb 2018, Thor Johnson wrote:

It does. My old patch to the firmware tried exposed that as a "4th axis" with the readout statements, but as it turns out, sys.feedrate has the last feedrate requested and didn't reflect the actual feedrates. This latest version computes it based on distance moved (so if you're doing very curvy, very fast, the velocity shown will be low since GC didn't take the curve into account when computing the velocity).

sus.feedrate isn't the right value (as you see), it would be something buried in the velocity PID loop

blurfl commented 6 years ago

The beauty of this approach is that it calculates the actual speed without using any of the processing power of the device it is monitoring.

krkeegan commented 6 years ago

Nothing in the PID has the desired linear velocity, the velocity controller relies on RPM.

Also, there is no guarantee that the requested RPM bears any basis in reality either. It can and does jump around quite a bit even with our improvements.

BarbourSmith commented 6 years ago

This one can be closed as "not needed."

Is that something I do or something the maintainers do?

Feel free to close anything that can be closed, especially if you opened it. We're a small enough group that I don't think we need any sort of hierarchy 😀