-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Geozones (aka Geofence) #10459
Geozones (aka Geofence) #10459
Conversation
|
Please enable by default for the SITL:
|
@stronnag Good catches. |
I have stuff I'd like to test against this ..... |
Looks good to me. Haven't test flown it. |
Just for the records: We are using the general code of Geozoning for nearly a year now in a commercial environment on the basis of INAV 7.1.2. So functional it works flawless. |
Is there a configurator counterpart? |
and
In the first instance In the past, we've tried to keep the in / out "structures" the same. This makes life easier for consumers. Otherwise they might assume the "structures" are the same, and massively corrupt the internal dataset (and then ask for a CLI command it fix the mess). |
For example, for Waypoints, Safehomes, FW Approach, we send and receive "structured item" elements in the same order. |
@stronnag |
Thanks. I will change mwp's implementation. |
Thanks for the input, I implemented your suggestions and added the possibility to specify minimum and maximum altitude as absolute altitude (sea level as refernz). |
that means INAV 8 will need the GTK4 version then, right? Just validating as I guess it will then not work with the GTK3 version anymore? |
Correct. Even before the review changes:
So the Gtk3 version cannot work with INAV8 (the FEATURE Id alone will prevent that, even before the new MSP (and then the changed new MSP). The current development branch mwp (GTK4) will work with INAV 8 (the state of this branch at time of writing). |
(erroneous report removed) |
OK, I'm stupid. Forgot to hard reset the SITL EEPROM after the last update. |
Any further changes needed here or any complaints? Or are we ready to throw it in? |
As far as configuration / MSP / round-tripping settings, I'm content that it's fit for purpose. |
Is there anything more Andy wants to do before merging (like push some documentation ....) ? |
Documentation should not be a big deal. If I remember right Andreas made a documentation for the Client version that should be just copy and paste but I am happy to help with that in a separate PR. |
Ah ok. :) I recommend NOT to configure geozones via CLI, it is very easy to create an invalid configuration (e.g. the vertices have to be |
@stronnag It is important that the polygons are arranged against the clock hand and in ascending order (parameter index in geozone vertex zone). The order in which they are stored is completely irrelevant (the config array can also be completely fragmented) as long as the index is correct. For the RTH algorithm it is also important that for overlapping zones there are at least two vertices of a zone with a minimum distance of 1 x loiter radius (fixed wings) or “geozone_mr_stop_distance” (multirotor) to the border of the other zone. |
I think some are needed. Previously, mwp had no rules about shape direction etc. If I uploaded the zones shown below to the FC, then downloaded them into the configurator then a Configurator upload would corrupt the FC dataset such that CLI Now mwp now performs some checks and cancels invalid uploads completely. I guess I could just reverse clockwise shapes automagically, but complete cancel might get the user to learn the rules ...
|
Maybe I missed something but why is the right most one invalid? The thick green one? Having U-Shaped or L-Shaped zones worked fine so far. |
@stronnag Marc is right, convex or concave zones are no problem, complex (i.e. with crossed lines) ploygons are not allowed. |
Because there is NO documentation on how this complex function is supposed to work and the check shown was too strict. |
Excellent! mwp zone checking is now working correctly (mea culpa). |
Not for F722 FC, as it is already full to the brim.
Documentation still to come :)