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

Shutdown ESCs when IDLE #97

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

olliewalsh
Copy link
Contributor

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)

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)
ene9ba added a commit to ene9ba/OpenMower that referenced this pull request Jul 29, 2024
@Apehaenger
Copy link
Contributor

Apehaenger commented Aug 17, 2024

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 ;-)

@rovo89
Copy link
Contributor

rovo89 commented Aug 18, 2024

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
Copy link
Contributor

rovo89 commented Aug 18, 2024

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.

@Apehaenger
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@rovo89
Copy link
Contributor

rovo89 commented Aug 18, 2024

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?

@Apehaenger
Copy link
Contributor

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 ;-)

@FadeFx
Copy link

FadeFx commented Aug 18, 2024

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?

@Apehaenger
Copy link
Contributor

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.

@rovo89
Copy link
Contributor

rovo89 commented Aug 18, 2024

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).

@olliewalsh
Copy link
Contributor Author

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

@rovo89
Copy link
Contributor

rovo89 commented Sep 8, 2024

@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 d432b54, only kill_sw_mode is relevant, right?

@patrickzzz
Copy link

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..

@rovo89 rovo89 mentioned this pull request Oct 12, 2024
12 tasks
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

Successfully merging this pull request may close these issues.

6 participants