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.

Sign up for GitHub

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

Jump to bottom

Barrier opening #5713

Merged
merged 27 commits into from
Jul 24, 2024
Merged

Barrier opening #5713

merged 27 commits into from
Jul 24, 2024

Conversation

Nick-Tallguy
Copy link
Contributor

@Nick-Tallguy Nick-Tallguy commented Jul 1, 2024 •
edited
Loading

Draft - ToDo Now all complete with icon

Requires a new icon - currently using icon from cycleway width New icon created and added

#5678 refers

Have tested with emulator on Android Studio, and have apk installed on my phone (Google pixel 8A).

This changeset by me on barrier = entrance.

First attempt at anything to do with kotlin or Android, so please check carefully & please let me know if there is anything I can do to improve - would like to contribute more in the future.

Nick-Tallguy and others added 7 commits June 25, 2024 14:45
@Nick-Tallguy
1st attempt at gates displaying only
55d619f
@Nick-Tallguy
Quest files created and mainly updated - still need to;
65c5fbc 
* Check values in Form,
* General check of files - although it builds, it will not run,
* New icon needed at some point,
* Need to filter out the gates etc which are on private roads etc - waiting until after the more general files are working first ..

issue streetcomplete#5678 refers
@Nick-Tallguy
Merge branch 'streetcomplete:master' into barrier_opening
c67d5a7
@Nick-Tallguy
update number applied to quest - had previously accidentally duplicat…
d1ac501 
…ed a number
@Nick-Tallguy
remove own copy of WidthAnswer.kt - not needed
d9642f1
@Nick-Tallguy
Barriers are now displaying - need to sort the filters. Emulator show…
d867ab4 
…s it is possible to update data - needs more checking.
@Nick-Tallguy
Flters seem to be working okay on emulator - need to produce apk and …
7498974 
…test on phone (emulator does not have streetmeasure, and won't seem to load it.

streetcomplete#5678  refers

Also still needs an icon.
@FloEdelmann FloEdelmann linked an issue Jul 1, 2024 that may be closed by this pull request
Barrier=gate - What is the available opening #5678
Closed
5 tasks
westnordost
westnordost requested changes Jul 3, 2024
View reviewed changes
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddBarrierOpening.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddBarrierOpening.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddBarrierOpening.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddBarrierOpening.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddBarrierOpening.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddGateWidthForm.kt Outdated Show resolved Hide resolved
Nick-Tallguy and others added 6 commits July 6, 2024 00:11
@Nick-Tallguy
Amendments to pull request;
7112015 
  * strings.xml - line 1775 question mark added, and line 1776 deleted.
  * AddBarrierOpening.kt -
  *   bollard and block removed,
  **   lines 59 - 62 - not changed yet (needs research by me),
  *   lines 66 - 70 - indentation added,
  **   lines 72 - 75 - not changed yet (needs research by me),
  *   line 39 CAR & PEDESTRIAN removed.

Will continue to research for items with ** & then resubmit.
@Nick-Tallguy
amend indentation to remove warning on Android Studio - Lint issue
1f7deea
@Nick-Tallguy
remove reference to removed string in Form
01fd90b
@Nick-Tallguy
remove reference to removed string in Form
36b6331
@Nick-Tallguy
remove unnecessary code & amend if / else statement
94dfa3d
@Nick-Tallguy
Merge branch 'master' into barrier_opening
3dc607f
@Nick-Tallguy Nick-Tallguy requested a review from westnordost July 11, 2024 16:45
Nick-Tallguy and others added 2 commits July 12, 2024 17:09
@Nick-Tallguy @westnordost
Update app/src/main/java/de/westnordost/streetcomplete/quests/barrier…
8c8032c 
…_opening/AddBarrierOpening.kt

Co-authored-by: Tobias Zwick <newton@westnordost.de>
@Nick-Tallguy @westnordost
Update app/src/main/java/de/westnordost/streetcomplete/quests/barrier…
bf7ef42 
…_opening/AddBarrierOpening.kt

Co-authored-by: Tobias Zwick <newton@westnordost.de>
@westnordost
Copy link
Member

Sorry to hear that. If you are unable to continue, I am sure someone else will be able to pick this up and bring this to completion as it seems to me that it is quite close to completion already.

@westnordost
Copy link
Member

Without having checked if there was a discussion about that beforehand, to me it appears like it makes more sense to always tag maxwidth on barrier openings rather than maxwidth:physical because the latter is usually only tagged if it differs from the signed maxwidth which is irrelevant here because the user is not asked for what is signed but is directly asked to provide the measured width.

@mnalis
Copy link
Member

mnalis commented Jul 13, 2024

The wiki seems pretty insistent that maxwidth exclusively "specifies a legal restriction of the maximum permissible width of a vehicle", and has a separate "common mistakes" section which warns about:

  • Do not confuse the specification of the maximum permissible width of vehicles with the specification of the actual width via width=*.

  • Do not confuse the specification of the legally restricted maximum width of vehicles with the specification of a physical width limit via maxwidth:physical=*.

@Nick-Tallguy
Copy link
Contributor Author

maxwidth:physical or maxwidth
I have been using maxwidth:physical because of the guidance in the wiki at https://wiki.openstreetmap.org/wiki/Key:maxwidth:physical which also appears in https://wiki.openstreetmap.org/wiki/Tag:barrier%3Dcycle_barrier#Using_dimension_attributes.
I can think of many instances in the UK and France where the local authority imposes a 'maxwidth except for access' restriction on a narrow road to avoid larger vehicles clogging up the roads. However, if you are the driver of a larger vehicle which has to make a delivery there, the physical dimension of the gateway or entrance is important and may necessitate using another approach to your destination.
To me maxwidth:physical is the option to go for.

@Nick-Tallguy
Copy link
Contributor Author

@westnordost & @mnalis
I feel that for this quest to be processed in a reasonable time it would be best if someone else finished it.

I like the fact that StreetComplete and SCEE are closely curated, and I intend to be involved, but I would rather not delay this quest any further by my lack of knowledge - I am studying, but have a way to go! At the moment my knowledge of kotlin is so poor that it will take a considerable time before I would be able to understand the code, and answer the queries.

Regards & thank you

@westnordost
Copy link
Member

Ok, you are right about maxwidth:physical

@Nick-Tallguy
remove last changes to AddWidthForm &
10cdf86 
remove 'GATEWAYS' - replace with list.
correct indentation errors.
westnordost
westnordost approved these changes Jul 24, 2024
View reviewed changes
Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Thank you for seeing it through!

app/src/main/java/de/westnordost/streetcomplete/quests/barrier_opening/AddBarrierOpening.kt Outdated Show resolved Hide resolved
@westnordost westnordost merged commit 42a5cd6 into streetcomplete:master Jul 24, 2024
1 check passed
@Nick-Tallguy Nick-Tallguy mentioned this pull request Aug 2, 2024
Closed
5 tasks
@Nick-Tallguy Nick-Tallguy deleted the barrier_opening branch October 25, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnalis mnalis mnalis left review comments

@westnordost westnordost westnordost approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Barrier=gate - What is the available opening
3 participants
@Nick-Tallguy @westnordost @mnalis