aerostitch / testnavit

0 stars 0 forks source link

[PATCH] more pois attributes #164

Open aerostitch opened 11 years ago

aerostitch commented 11 years ago

Issue migrated from trac ticket # 1084

component: tools | priority: major | keywords: binfile, openstreetmap

2012-12-02 09:42:14: erfabbri@gmail.com created the issue


im a newbie, i managed to add more attributes for pois when clicking on "show pois attributes", modifing osm.c and attr_def.h

in short, this patches will add more poi info like:

  • opening_hours
  • capacity
  • cuisine,stars(used in hotels)
  • operator as the latest version of navit only include address, phone, fax and postal code

some of my implementations seems to be nearly supported by navit, as i saw in osm.c the tag "addr:email", that i fixed with the current osm tag "email"

also some of the tags i added to be displaied, were already in attr_def.h like: ATTR(description) and ATTR(url)

also ATTR(entry_fee) and ATTR(open_hours) were already in, but i changed in ATTR(fee) [more generic fee for an amenity, not only entrance] and in ATTR(opening_hours) [more close to the osm tag]

NOTE: adding ATTR(operator) in attr_def.h, give the word operator in red color, using gedit.

as a newbie in C language, i dont know if this is a bug/problem and how to avoid this, so i used ATTR(operated_by) to solve.

i compiled and it worked, the only problem is about fuel:lpg=yes/no or fee=yes/no....it appears fee=1 when yes and fee=0 when no...but user tryagain in irc said you will fix this ;)

if it doesnt makes world binfile grow too much, can you include in mainstream?

thanks

aerostitch commented 11 years ago

2012-12-02 09:43:19: erfabbri@gmail.com uploaded file osm.c.patch (4.0 KiB)

aerostitch commented 11 years ago

2012-12-02 09:43:32: erfabbri@gmail.com uploaded file attr_def.h.patch (0.5 KiB)

aerostitch commented 11 years ago

2012-12-04 13:30:14: tryagain commented


Hi!

I have not yet considered your patch in its merits, but have some common suggestions.

  1. It's better to make your patches with 'svn diff' command while you're in the root of the project source tree. Then you have all changes in one file and it's easier for devs to apply and review them.

  2. Changing the spelling of existing attributes is not generally a good idea as somebody can have their names in navit.xml, text file maps etc. If you really need to change the spelling, it's usually better to add a new attribute than change existing one.

tryagain.

aerostitch commented 11 years ago

2012-12-04 13:30:14: tryagain

aerostitch commented 11 years ago

2012-12-04 13:30:14: tryagain

aerostitch commented 11 years ago

2012-12-05 04:38:12: erfabbri@gmail.com commented


added svn diff generated patch as suggested, and old ATTR are not removed now.

ps: for key=yes/no (fee,fuel:lpg,dispensing,atm)....still appears key=1 when yes and key=0 when no in Navit, please fix.

thanks.

aerostitch commented 11 years ago

2012-12-05 04:38:12: erfabbri@gmail.com

aerostitch commented 11 years ago

2012-12-07 03:42:17: erfabbri@gmail.com uploaded file svn_diff.patch (5.2 KiB)

aerostitch commented 11 years ago

2012-12-16 05:11:04: cp15@navit-project.org commented


The patch has a few problems. First it is not allowed to insert new attributes in the middle of a block. It will make maps incompatible, since the numbering of the following attributes will change and the attributes are represented in the map as numbers

Then the attributes are not expected to be user-readable in the first place, but instead should describe the object as good (and abstract) as possible. In particular:

  • Capacity is most likely a numeric attribute and not a string
  • Fee is a boolean value or time dependent, I would split this up into fee (yes/no) and fee_hours (string)
  • fuel:lpg is a bit arbitrary. I would like to use a bit field for fuel stations where each fuel type has a bit
  • description is ok I think
  • operated_by: I am not sure whether this should be a separated attribute or coded into the item types (like poi_fastfood_mcdonalds), at least for the more important ones. This would allow to display a brand icon on the map and do more specific searches.
  • cuisine: This needs to be more formal than a text string. Maybe it should be also integrated into the item type (poi_restaurant_italian) for specific searches.
  • opening_hours: open_hours should be sufficient for this, no need to introduce a new tag. In the long term I would like to convert the osm style intervals into gdf timespecs which are much more formal.
  • stars: I think it is also a numerical attribute... To include the 'Superior' suffix, I would multiply the number of stars by 2 and add 1 for superior.
  • dispensing: I would code this into a new item type poi_pharmacy_nondisp
  • atm: Should be coded as poi_atm

Best regards,

cp15

aerostitch commented 11 years ago

2012-12-18 04:32:48: erfabbri@gmail.com commented


Replying to [comment:3 https://wiki.navit-project.org/index.php/user:cp15]:

The patch has a few problems. First it is not allowed to insert new attributes in the middle of a block. It will make maps incompatible, since the numbering of the following attributes will change and the attributes are represented in the map as numbers

Then the attributes are not expected to be user-readable in the first place, but instead should describe the object as good (and abstract) as possible. In particular:

  • Capacity is most likely a numeric attribute and not a string

that's true...in osm taginfo only some values like "unknow" is a string but useless, so i agree

  • Fee is a boolean value or time dependent, I would split this up into fee (yes/no) and fee_hours (string)

fee_hours is not in use http://taginfo.openstreetmap.org/search?q=fee_hours also "fee" in OSM seems not to be only a boolean: http://taginfo.openstreetmap.org/keys/fee#values a 5% of informations will be lost with boolean

  • fuel:lpg is a bit arbitrary. I would like to use a bit field for fuel stations where each fuel type has a bit

so you will introduce also other fuel type like biofuel and natural gas?

  • description is ok I think

i agree

  • operated_by: I am not sure whether this should be a separated attribute or coded into the item types (like poi_fastfood_mcdonalds), at least for the more important ones. This would allow to display a brand icon on the map and do more specific searches.

well, this can be made but as an extra feature. also for McDonald's i will not use operator but brand tag (as franchising). the puropose of my patch is to visualize the operator=* value without lost of informations.

  • cuisine: This needs to be more formal than a text string. Maybe it should be also integrated into the item type (poi_restaurant_italian) for specific searches.

ok, but what about cases like: "cusine=pizza;kebab;burger" ?

  • opening_hours: open_hours should be sufficient for this, no need to introduce a new tag. In the long term I would like to convert the osm style intervals into gdf timespecs which are much more formal.

i agree to use open_hours

  • stars: I think it is also a numerical attribute... To include the 'Superior' suffix, I would multiply the number of stars by 2 and add 1 for superior.

for me is fine, but i read in OSM ML there is a discussion about this tag, they are pushing to use "award:hotelstars" ( "hotelstars" is a european certified system that assign stars to hotels in a more comparable way) so maybe we can postpone this attributes for now?

  • dispensing: I would code this into a new item type poi_pharmacy_nondisp

we should also consider pharmacy without dispensing=* tag as unknow?

  • atm: Should be coded as poi_atm

agree...but in OSM there are still 2.323 post_office and 63.217 bank with atm=* so maybe is still a usefull information

Best regards,

cp15

thanks for possible future implementation cheers.

aerostitch commented 11 years ago

2013-03-18 05:56:21: erfabbri@gmail.com commented


just notice in attr_def.h the line: ATTR(route_follow_straight_REMOVE) // This is to be removed with the next version

and also ATTR(town_id), ATTR(street_id), ATTR(district_id), and ATTR(trackingo) have a comment / fixme /

maybe you want to make a "cleaning" on attr_def.h ?

aerostitch commented 11 years ago

2013-07-14 01:15:38: usul changed component from core to tools

aerostitch commented 11 years ago

2013-07-14 01:15:38: usul edited the issue description

aerostitch commented 11 years ago

2013-07-14 01:15:38: usul

aerostitch commented 11 years ago

2013-07-14 01:15:38: usul commented


Was the patch improved yet?

As it seems to be on the way, I schedule it for 0.5.1 hotfix.

aerostitch commented 6 years ago

2017-12-02 04:08:44: @jkoan

aerostitch commented 6 years ago

2017-12-02 04:08:44: @jkoan commented


This ticket was pushed back in order to bring 0.5.1 out soon.