Skip to content

Comments

Correct curvature check and respect options#32

Open
CarawaySeed42 wants to merge 2 commits intomasterfrom
Curvature-Check
Open

Correct curvature check and respect options#32
CarawaySeed42 wants to merge 2 commits intomasterfrom
Curvature-Check

Conversation

@CarawaySeed42
Copy link
Contributor

The local curvature check was incorrectly implemented and only worked in certain circumstances.
Changes to crg read and write were necessary to properly respect the global curvature check option.
The curvature check also did not follow the specification and the specification incorrectly stated the default global curvature options.
This implementation leaves the curvature check working as it was intended
but the intention is up for discussion.

Resolves: #8

Signed-off-by: Patrick Kuemmerle patrick.kuemmerle@3d-mapping.de

The local curvature check was incorrectly implemented
and only worked in certain circumstances.
Changes to crg read and write were necessary to properly
respect the global curvature check option.
The curvature check also did not follow the specification
and the specification incorrectly stated the default
global curvature options.

Resolves: #8

Signed-off-by: Patrick Kuemmerle patrick.kuemmerle@3d-mapping.de
@CarawaySeed42 CarawaySeed42 self-assigned this Oct 29, 2025
@CarawaySeed42 CarawaySeed42 added isType:Bug An issue that contains contradictions or errors in the standard. isState:ReadyforCCBreview CCB will review it and change the status to ReadyForMerge if everything is ok labels Oct 29, 2025
@CarawaySeed42 CarawaySeed42 added this to the v1.2.2 milestone Oct 29, 2025
@asadekasam asadekasam requested a review from a team November 3, 2025 09:18
The ok status was always reset even if the local check succeeded.
This triggers another unnecessary crg check later.

Resolves: #8

Signed-off-by: Patrick Kuemmerle <patrick.kuemmerle@3d-mapping.de>
@3Dbastian 3Dbastian mentioned this pull request Dec 10, 2025
@3Dbastian
Copy link
Contributor

3Dbastian commented Dec 10, 2025

Considerations regarding this pull request:

  • Please see also general idea of the local curvature check in Local Curvature Check #8
  • The proposed changes were necessary, since the newly implemented local check only worked under certain situations
  • Actually the Flags WARN_GLO and WARN_LOC are no warnings but errors, if they are activated the Writing and Plotting of a CRG fails in the Matlab API causing a Matlab error, Reading works
  • There are four possible combinations of setting the flags
    • WARN_GLO = 0 and WARN_LOC = 0 -> no Check
    • WARN_GLO = 1 and WARN_LOC = 0 -> only Global check = Default
    • WARN_GLO = 0 and WARN_LOC = 1 -> not useful -> proposal if this is used WARN_LOC is also set to 0 (alternative would be to set WARN_GLO to one)
    • WARN_GLO = 1 and WARN_LOC = 1 -> both checks are activated
  • crg_test_curvature.m example was fixed, please note, the road used for that example is only for showing the curvature vs width effect, but is not intended to have meaningful height values

Copy link
Contributor

@3Dbastian 3Dbastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • in my opinion the local check is a legitimate feature
  • the changes are necessary to make it fully functional for Matlab API

@CarawaySeed42
Copy link
Contributor Author

CarawaySeed42 commented Dec 15, 2025

Also see commit 8261069 to understand why statements regarding the curvature check option flags are phrased the way they are.

If
WARN_GLO == -1 (default if not specified in file) then it is implicitly treated as unequal to 0 -> do check
If
WARN_LOC == -1 (default if not specified in file) then it is implicitly treated as smaller or equal to 0 -> do not check

@asadekasam asadekasam requested a review from a team January 12, 2026 12:16
@CarawaySeed42 CarawaySeed42 marked this pull request as ready for review February 2, 2026 09:05
@asadekasam asadekasam requested a review from jorauh February 9, 2026 12:35
@asadekasam
Copy link
Contributor

OpenCRG CCB 09.02.2026: @jorauh will review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isState:ReadyforCCBreview CCB will review it and change the status to ReadyForMerge if everything is ok isType:Bug An issue that contains contradictions or errors in the standard.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local Curvature Check

3 participants