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

Allow level information on PathLinks #239

Merged
merged 4 commits into from
Oct 11, 2022

Conversation

ue71603
Copy link
Contributor

@ue71603 ue71603 commented Aug 26, 2022

Allows for level information on PathLinks. This PR responds to #145.

#145 asks for more data to tell a passenger, for instance, to use an elevator leading from level "A" to level "U1". This is achieved by adding FromLevel, ToLevel, LevelNameStructure.

Typical attributes useful for guidance in that kind of use cases (example):
PathLink.Transition: down
PathLink.AccessFeatureType: elevator
PathLink.FromLevel.PublicCode: A (new)
PathLink.ToLevel.PublicCode: U1 (new)

Two other PRs related to this will follow soon - dealing with real-time information about access equipment (#194) and additional accessiblity information.

Replaces #234

Aurige
Aurige previously approved these changes Aug 26, 2022
Copy link
Contributor

@sgrossberndt sgrossberndt left a comment

Choose a reason for hiding this comment

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

Instead of repeating Level in From->LevelPublicCode and From->LevelName and having From->Id what about naming From FromLevel and To ToLevel and thus having FromLevel->PublicCode, FromLevel->Name, FromLevel->Id instead? This PublicCode part would be more consistent with the just merged simplified element names

@@ -743,6 +743,38 @@
<xs:documentation>Number how often the access feature occurs in this PathLink</xs:documentation>
</xs:annotation>
</xs:element>
<xs:element name="From" type="PathLinkEndStructure" minOccurs="0">
Copy link
Contributor

Choose a reason for hiding this comment

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

This structure makes it possible to only supply a From, but no To. Maybe add a group consisting of exactly one From and one To element and give the group minOccurs="0" instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

@sgrossberndt The structure also makes it possible to supply only a To. But wouldn't that be welcome? - When giving directions, it is usually sufficient to provide information only for the To (for instance the PublicCode of the target level when using an elevator).
Regarding your previous comment: The idea of the Id was not to point to a Level, but more generally to an element at the end of a PathLink - for instance a NeTEx AccessSpace or an EquipmentPlace (this would later allow for retrieving additional information, e.g., the coordinates of the EquipmentPlace of the elevator). Thus, ToLevel->Id would not be what was intended. That said, I think it is better to stay with the proposed solution. What do you think?

OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
trurlurl
trurlurl previously approved these changes Sep 2, 2022
Copy link
Contributor

@trurlurl trurlurl left a comment

Choose a reason for hiding this comment

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

OK. I think the change requested by @sgrossberndt is obsolete, see the first "show comment" in the conversation tab.

OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
@trurlurl
Copy link
Contributor

Documentation updated accordingly.

trurlurl
trurlurl previously approved these changes Sep 20, 2022
Aurige
Aurige previously approved these changes Sep 20, 2022
skinkie
skinkie previously approved these changes Sep 21, 2022
sgrossberndt
sgrossberndt previously approved these changes Oct 11, 2022
@ue71603
Copy link
Contributor Author

ue71603 commented Oct 11, 2022

We will use kilograms. Also use float

@sgrossberndt sgrossberndt dismissed stale reviews from skinkie, Aurige, and trurlurl via c751458 October 11, 2022 07:56
@sgrossberndt sgrossberndt force-pushed the AllowsLevelInformationOnPathLinks branch from 4d95e41 to c751458 Compare October 11, 2022 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants