feat(autoware): blueprint MGRS-offset fallback for sensor.other.autoware_gnss - #38
Draft
youtalk wants to merge 1 commit into
Draft
feat(autoware): blueprint MGRS-offset fallback for sensor.other.autoware_gnss#38youtalk wants to merge 1 commit into
youtalk wants to merge 1 commit into
Conversation
…are_gnss The Autoware GNSS sensor reads its MGRS offset only from the level's AutowareWorldSettings -> UMgrsDataAsset. A level that has no such asset -- any imported digital twin, and every map a user cannot re-save -- publishes /sensing/gnss/pose in an unoffset frame, so on an MGRS lanelet2/PCD map Autoware's initial pose lands tens of kilometres off the map, where NDT cannot be expected to initialise. UE's RepairWorldSettings gives such a level an AutowareWorldSettings with an empty pointer at load time, so the content path cannot be reached without the editor. Add mgrs_offset_x/y/z (Float, metres, Autoware map frame) to sensor.other.autoware_gnss, read only when the level provides no data asset; the asset stays primary and a non-zero fallback beside it is logged and ignored. Parse the wire values as double: an offset of ~1e5 m would resolve to ~8 mm as float. autoware_demo.py gains --mgrs_offset and run_carla_autoware.sh forwards --map-origin to it. A gtest pins the CARLA->ROS Y mirror and the Nishi golden offsets shared by every consumer of this offset. Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem.
AAutowareGnssSensorreads its MGRS offset from exactly one place: the level'sAAutowareWorldSettingsand theUMgrsDataAssetits soft pointer names. A level with no such asset publishes/sensing/gnss/pose_with_covariancein an unoffset frame, which on an MGRS lanelet2/PCD map puts the published pose tens of kilometres outside the map — measured below at roughly 81 km on Nishi-Shinjuku. That is not an edge case for imported content: any digital twin brought in as OpenDRIVE plus lanelet2 arrives without the asset, and a user who cannot re-save the level has no way to add one.Why this cannot simply be fixed content-side. The obvious answer is "wire the asset into the level", and that remains the primary and preferred path — it is what the data asset is for. But the level's soft pointer is empty at load time even on a level that once had it set, because UE's
RepairWorldSettingsre-creates theAWorldSettingsobject without carrying the pointer over. Captured directly, before any wiring, in~/ue58-logs/p3/08-summary.md:World settings (before apply): class: AutowareWorldSettings, soft_ptr: None. Re-wiring requires the editor. The runtime alternative in this PR is what makes such a map usable without one.Change. Add three
Floatattributes —mgrs_offset_x,mgrs_offset_y,mgrs_offset_z, metres in the Autoware map frame — tosensor.other.autoware_gnss, and read them only when the level provides no data asset.LoadMgrsData()is restructured so that the asset stays primary: if the asset is present it is used, and a non-zero blueprint value beside it is logged and explicitly ignored. Four log lines, one per branch, make the taken path greppable in the server log.autoware_demo.pygains--mgrs_offsetandrun_carla_autoware.shforwards--map-originto it, so the value already needed by the goal conversion and by gate 1 reaches the sensor as well instead of being typed twice.CHANGELOG.mdgains a matching bullet at the top ofLATEST Changes— this adds three public blueprint attributes to a shipped sensor and touchesPlugins/Carla/Source/**, the same class of change as the existing--ros-domain-idand Zenoh-middleware entries.The wire values are parsed as
double, deliberately.FActorAttributestores the attribute as a string and range-validates it throughAs<float>(), but the value is read withFCString::Atod, so the full decimal precision survives to the sensor. This matters at MGRS magnitudes: an offset of ~1e5 m has afloatquantum of about 8 mm, which is the same order as the residual the precedence probe below measures, and it would put a floor under localisation accuracy for no reason.Two behaviours the rewrite changes that a reviewer should see stated rather than discover. First, the pre-existing
MGRS Data Asset SoftPtr not set in WorldSettings.warning now also fires when theAAutowareWorldSettingscast fails, a case that was previously silent. That is a strict superset of the old condition and is an improvement — a level whose world settings are the wrong class is exactly a case worth a diagnostic — but it is a behaviour change on the log. Second, the rewrite silently closes a latent bug that no test covered: the old code assignedMgrsDataAssetunconditionally afterLoadSynchronous(), so a broken soft reference left the pointer null with no diagnostic at all. The new flow falls through to the warning.Where has this been tested?
CarlaUnreal/UnrealEngine@ue58-dev-carlacacb25b99f14. The three plugin files were compiled in a local UE 5.8 editor build with zeroerror:lines and no diagnostic naming any of them; no fork CI job compiles them.Before, on the pristine level (
~/ue58-logs/p3/03-gnss-before.txt, and~/ue58-logs/p3/02-server-grep.txtline 3 — that file is three lines long, and the4763:it carries inside is agrep -nprefix from the simulator log the grep was taken over, which later probes have since overwritten):/sensing/gnss/pose_with_covariancex=-279.058 y=-220.875 z=0.545x=-279.059 y=-220.875 z=0.545— matchesx=81376.671 y=49916.555 z=43.045— no matchMGRS Data Asset SoftPtr not set in WorldSettings.in the server logThe "before" column is one topic sample plus one log line, not a failed run, and must not be read as if the gates had been measured with the offset absent. What was measured is the published pose and the server-log warning. No cell in this work ever ran the gates in that state: the fallback cell is a pristine level with the fallback supplying the offset, and the other two cells are wired. That NDT would fail to initialise roughly 81 km outside the point-cloud map is a reasoned expectation from the measured displacement, not a result; measuring it would need a further cell with the offset deliberately suppressed, which was not run.
After — the same map in both level states, each driving a full closed loop to ARRIVED. Both cells used identical map, route, poses, build and harness; the only difference is which source supplies the offset.
N-content(~/ue58-logs/p3-10-cell-nishi-content/)N-fallback(~/ue58-logs/p3-11-cell-nishi-fallback/)N-humble(~/ue58-logs/p3-12-cell-nishi-humble/)Which branch ran, per cell — one
gnss-source.txtper cell directory, corroborated in bothcarla_server.logand the cell's copy of the engine log at the same timestamp and frame. With the level wired (N-content), two lines at the identical timestamp:With the level pristine (
N-fallback):In the two wired cells
MGRS Data Asset SoftPtr not setappears 0 times; in the fallback cell so do both data-asset lines, and the pre-existing warning is correctly suppressed rather than fired alongside a working offset.N-humblereproduces the asset path to six decimals.The offset reaches Autoware's map frame, not merely a log line. The engage precheck compares CARLA ground truth against Autoware's own
/localization/kinematic_statebelief, and prints the same result in all three cells (…/automation.log):GATE1: truth (81376.10, 49916.10) yaw 34.0 deg | belief (81376.13, 49916.12) yaw 34.0 deg | delta 0.04 m, 0.0 deg. The belief side is computed entirely inside the stack, so this is an origin-independent second witness, and it is the one that shows propagation rather than branch selection. The two kinds of evidence are complementary and both belong here: the precheck cannot say which source supplied the offset, and the grep cannot say the value propagated. The agreement is identity at the printed precision of 2 decimal places — 1 cm — so it is not bitwise identity.Precedence under disagreement, proved behaviourally. In the two gate cells the fallback carried a value identical to the asset's, so those cells cannot distinguish "the asset won" from "either source would have produced the same number". A dedicated probe closes that gap (
~/ue58-logs/p3-10b-precedence/): wired level, plus a deliberately divergent--mgrs_offset=1000,2000,3000, about 94 km away.(81375.952382, 49916.583549, 43.035453)(81375.952382, 49916.586601, 43.035328)(720.222382, 1779.156601, 3000.535348)The blueprint value was genuinely supplied —
mgrs_offset_x/y/z has_attribute = Trueon the live server (…/blueprint-attrs.txt) — and 1000/2000/3000 appear nowhere in the level. Both log lines now carry different numbers, which is what makes the "ignoring blueprint mgrs_offset" line load-bearing instead of echoing the asset's own values as it did in the gate cells. The ~3 mm residual is a tick-pairing artefact rather than offset error: the ROS publish and the MGRS arithmetic use the sameSensorWorldTransformcaptured within one tick, and the residual is the external client'sget_transform()landing on a later tick while the ego micro-settles; the later of the two paired samples has the smaller residual (3.05 mm against 10.58 mm). Sensor noise is ruled out independently —noise_*_stddevand*_biasall default to0.0f, the demo never sets them, and the published covariance is all zeros.The claim that evidence licenses, scoped. Verified on Nishi-Shinjuku with
sensor.other.autoware_gnssgiven a deliberately divergent blueprint fallback (1000/2000/3000 m, ~94 km away): the server log confirms the data-asset offset was read and the blueprint offset explicitly ignored, and the published/sensing/gnss/pose_with_covariancematches the asset candidate to ~3 mm while sitting ~94 km from the blueprint candidate.AAutowareGnssSensor::LoadMgrsData()is a pure presence check —if (MgrsDataAsset) { …; return; }followed byif (bHasMgrsOffsetFallback)— with no magnitude or sign comparison anywhere, so the fact that only one direction of disagreement was measured is a weaker limitation than n = 1 usually implies: no code path exists in which the relative size or sign of the two offsets could change which branch wins. A bare universal "the asset always takes precedence" is not asserted.Unit and gtest coverage, run locally.
libcarla_test_serverandlibcarla_test_clienteach report 10 tests from 2 suites, 10 passed — 3 newAutowareGnssOffsettests plus the 7 pre-existingTransformQuaterniontests (~/ue58-logs/p3/13-gtest.txt). Read that coverage honestly: the new test reproducesAutowareGNSSPublisher::Write's composition rather than invoking it, so a reordering insideWrite()would leave it green; what it does pin is theTransformQuaternion.hY-mirror and the Nishi golden offsets that every consumer of this offset shares. The test file says so in its own header comment. Two Python tests for--mgrs_offsetparsing were added underPythonAPI/test/unit/and pass locally; no fork job runs that directory.The new C++ is provably live in the binary that ran the cells, which is the trap this project has hit before — a green build can silently run a stale editor module.
nm -DC …/Plugins/Carla/Binaries/Linux/libUnrealEditor-Carla.so | grep -c SetMgrsOffsetFallbackreturns 1, and the module.sois timestamped inside that build's window (~/ue58-logs/p3/13-build-editor.log). Usenm -DC: plainnm -Creports "no symbols" on this.so.Limits of the live evidence, stated plainly. n = 1 per arm: one map, one offset value, one build, Fast DDS only; the ROS distro is the only axis with two samples. Nothing here says anything about cruise-phase localisation — the gate harness runs G2, then a settle, then G3, then G1, so every G1 window in this work opens about 52 s into a ~62 s route and covers the terminal ~31 m of a 221 m drive at around 1 m/s mean. Those windows are genuine in-motion data from an unmodified harness, but each cell's G1 maximum is a transient two-sample spike, not a tracking-quality measure, and the same configuration produced 0.055 m and 0.198 m in two runs differing only in where the window was placed — a same-config spread wider than the 0.059 m difference between the asset and fallback cells. The correct reading of that comparison is that the fallback is not a regression on the asset path, not that the two are equivalent to within a few centimetres.
Possible Drawbacks
Warninglevel with both values printed, so the situation is visible in the server log rather than silent. The alternative — letting the blueprint override — would let a client silently move a level's georeference, which is worse.mgrs_offsetof0,0,0is indistinguishable from "not supplied".SetMgrsOffsetFallbacksetsbHasMgrsOffsetFallback = !OffsetMeters.IsNearlyZero(), so an explicit all-zero fallback emits neither log line and falls through to the pre-existing warning. On a Local-projector map a zero offset is also the correct behaviour, so this is arguably right, but it is untested by any cell and it is a diagnostics wart: the offset applied is{0,0,0}either way, so no published output differs.mgrs_offsetproduces a confidently wrong pose, and the symptom is a localisation failure rather than a configuration error. The same is true of the--map-originvalue earlier in this series; forwarding one to the other at least means the value is typed once.0,0,0, so--map-origin "0.0,0.0,0.0"would be forwarded rather than suppressed. Harmless — the server maps a zero fallback back to "not supplied" — but the guarantee is spelling-based, not numeric.--mgrs_ois no longer an unambiguous abbreviation of the pre-existing--mgrs_off, now that--mgrs_offsetexists, and the two flags read as near-opposites: one disables MGRS handling, the other supplies the offset. Renaming either is a wider change than this PR should carry, but the collision is worth knowing about.BUILD_CARLA_UNREAL=OFF, so the only compile evidence forAutowareGnssSensor.{h,cpp}andActorBlueprintFunctionLibrary.cppis the local UE 5.8 editor build cited above. The upstream self-hosted workflow does build the Unreal project and would cover them, but it does not run on this fork.