OpenMower icon indicating copy to clipboard operation
OpenMower copied to clipboard

Shutdown ESCs when IDLE

Open olliewalsh opened this issue 1 year ago • 14 comments

Align IMU accelerometer axes. Calculate pitch, roll, and overall tilt angles.

Set ESC shutdown pin to HIGH if:

  • not on a slope (pitch < 15deg) and...
  • ROS is running and...
  • current mode is IDLE

Requires killswitch to be set to ADC2_HIGH in xesc app config

(Based on the work of @Pete_MI632 on discord)

olliewalsh avatar May 18 '24 11:05 olliewalsh

~~Forget all I wrote... didn't recognized that it already got adapted and merge :-( What a pain.~~

Interesting PR! Planned to test it today, but after I reviewed it once more, I've some brain-blocker. Think it's due to my limited brain- resources, so please be so kind and help my rusty brain to get on track ;-)

Set ESC shutdown pin to HIGH if:

* not on a slope (pitch < 15deg) and...

* ROS is running and...

* current mode is IDLE

I don't get the logic behind this (as well as in the code). Did I understood right that "ESC shutdown pin = high" is "ESC Power status = off

If so: Why shutdown ESC when not on a slope?

What's the reason for shutting down the ESC's dependent on slope? Is it some kind of safety, but without triggering emergency? If so, why not also triggering on roll(/tilt?) angle? Guess it's due to some IMU miss-understanding by me. I'm here when talking about pitch/roll https://atadiat.com/en/e-towards-understanding-imu-basics-of-accelerometer-and-gyroscope-sensors/ section "Computing the Pitch and Roll Angles Using Accelerometer"

If "ESC shutdown" get triggered due to pitch(/roll), also the drive ESCs get shutdown right? Means we've a mower standing around without recognizing, because no emergency got triggered?

Please don't understand me wrong. I do like the PR but I miss somehow (parts of) the reasoning behind ;-)

Apehaenger avatar Aug 17 '24 17:08 Apehaenger

If so: Why shutdown ESC when not on a slope?

My understanding is that on a slope, the mower might roll off the docking station. So we need the motors to break actively, for which we need the ESC to be powered on.

rovo89 avatar Aug 18 '24 17:08 rovo89

Maybe you missed the point that this is for power saving and less heat while the mower is docked, not to stop the mowing motor in a potentially dangerous situation.

rovo89 avatar Aug 18 '24 17:08 rovo89

Wohoo! That's the part I missed ;-). But wait ... with that new reason, it would (actively) break only if >15° pitch, isn't it? Well the logic is clear then, but who the hack put his docking station on a >15° slope :-/ Because 15° is really a lot!! I guess the mower will roll out of his dock at >= 5°?!

Apehaenger avatar Aug 18 '24 17:08 Apehaenger

Maybe you missed the point that this is for power saving and less heat while the mower is docked, not to stop the mowing motor in a potentially dangerous situation.

Yes, I missed that fully!!

Anyway, I do like the idea to use the IMU for it and I think we can/should use it for both reasons?

Apehaenger avatar Aug 18 '24 17:08 Apehaenger

Like: If docked (VCharge) && pitch > 5° enable ESC_Power (for active break) || If !docked && !idle && >15° => disable ESC for safety and trigger emergency?

Apehaenger avatar Aug 18 '24 17:08 Apehaenger

If !docked && !idle && >15° => disable ESC for safety and trigger emergency?

Can't really judge because my lawn is almost flat. I think some users have a steeper lawn though, and as long as the hall sensors don't raise an alarm, they probably don't want it to trigger emergency mode. And shutting down the wheel ESCs would be a bad idea in that case as well. Maybe it might make sense the other way around: If emergency is triggered, the mower ESC could be turned off, and also the wheel ESCs if it's not steep.

That said, I think it would be a completely different intention, different implementation and to be discussed with other affected users, so I would separate it from this PR.

I guess from your additional commits, the only thing to keep would be using the constant?

rovo89 avatar Aug 18 '24 19:08 rovo89

If !docked && !idle && >15° => disable ESC for safety and trigger emergency?

Can't really judge because my lawn is almost flat. I think some users have a steeper lawn though, and as long as the hall sensors don't raise an alarm, they probably don't want it to trigger emergency mode. And shutting down the wheel ESCs would be a bad idea in that case as well. Maybe it might make sense the other way around: If emergency is triggered, the mower ESC could be turned off, and also the wheel ESCs if it's not steep.

Good point. Mine is also nearly 15° I guess, but didn't tested in real yet, that's why I didn't increased that value (with my wrong reasoned assumption). You're right, could be bad to shutdown the ESCs on a hill... even though it shouldn't trigger with a reasonable value like 20-25°, except one lift it from the back or side, in which case. Bla bla ... ;-)

That said, I think it would be a completely different intention, different implementation and to be discussed with other affected users, so I would separate it from this PR.

Okay, no problem. But >15° as roll-out undock protection look really very high to me.

I guess from your additional commits, the only thing to keep would be using the constant?

Take or drop whatever you (dis)like ;-)

Apehaenger avatar Aug 18 '24 19:08 Apehaenger

Guys, i am not sure, but i think you have a map of the yard in memory, you probably should also have stored surface angles, when taking the map. You also now exactly the possition of the mower at any time. Couldnt you just say turn shit off, if the angle of the mower differes a certain ammount from mapped angle?

FadeFx avatar Aug 18 '24 21:08 FadeFx

Guys, i am not sure, but i think you have a map of the yard in memory,

No, not in LowLevel ;-)

you probably should also have stored surface angles,

No, don't think so. As far as I know our map is 2D

when taking the map.

I'm sorry to say: "No not in LowLevel" ;-)

You also now exactly the possition of the mower at any time.

Unfortunately not "at any time". We can't expect GNSS data in docking station

Couldnt you just say turn shit off, if the angle of the mower differes a certain ammount from mapped angle?

No ;-). But luckily that's not the question anymore.

Apehaenger avatar Aug 18 '24 21:08 Apehaenger

you probably should also have stored surface angles, when taking the map

This is quite off-topic, but when recording the map, you only drive the outline of the area and obstacles. So there's no knowledge about any hills in the middle of the area. Besides that, the angles obviously depend on the mower orientation, so we'd have to be very sure about that one. I'd say it only makes sense to trigger emergency based on IMU for really extreme values (like: nose pointing down, mower upside down).

rovo89 avatar Aug 18 '24 22:08 rovo89

It will shutdown the ESCs when the mower status is IDLE (so docked or when stuck in emergency, although I think it will now stay mowing/PAUSED instead of failing to IDLE so need to update this logic), and not on a slope so it doesn't roll into a pond if it's stuck in emergency

olliewalsh avatar Aug 19 '24 10:08 olliewalsh

@olliewalsh Any idea how to verify that it works properly? The only indicator I have right now is that the docking station consumes ~0.6W less when the mower is docked and fully charged. I bought an IR thermometer, but the measured temperature varies so much even under seemingly stable conditions that I don't trust it.

From d432b545fae765966e4e3cc7ab3296d08dee9738, only kill_sw_mode is relevant, right?

rovo89 avatar Sep 08 '24 19:09 rovo89

One sign that it works is, that you are able to turn the wheel when idle (if not on slope).. when picking up back on a higher angle, wheels should brake..

Btw. I checked behavior of stock mower: it has the brake activated in docking station in flat ground.. although for me it sounds reasonable not to use the break when not needed..

patrickzzz avatar Sep 08 '24 20:09 patrickzzz

i think you have a map of the yard in memory, you probably should also have stored surface angles

no, map is 2d

olliewalsh avatar Apr 12 '25 10:04 olliewalsh