gluon-config-mode-geo-location-with-map: integrate a location picker map #1484
Conversation
|
Thank you for your work. We will surely have users entering more accurate coordinates with this. |
|
On 07/18/18 22:47, Vincent Wiemann wrote:
The map should only shown if the clients computer has an internet connection.
There are multiple packages in one Makefile. I was discussing with @NeoRaider about various solutions and after few try`s I was landed on this solution. Which has less redundancy code and backward compatibility for people who would preferring the current solution without map. |
|
@CodeFetch Now the map is only visible on internet connection. |
rotanid
added
0. type: enhancement
| @@ -6,8 +6,62 @@ PKG_VERSION:=1 | |||
| include ../gluon.mk | |||
|
|
|||
| define Package/gluon-config-mode-geo-location | |||
| TITLE:=Set geographic location of a node | |||
| TITLE:=Set geographic location of a node manualy and share it. | |||
| DEPENDS:=+gluon-config-mode-core +gluon-node-info | ||
| endef | ||
|
|
||
| define Package/gluon-config-mode-geo-location-with-map | ||
| TITLE:=Set geographic location of a node manualy and share it, optionally show a map if internet available. |
There was a problem hiding this comment.
"manually" with two "l"
"if internet is available" (adding the "is")
| "Koordinaten. Hier hast Du die Möglichkeit, " | ||
| "die Koordinaten händisch zu hinterlegen." | ||
| #ifdef WITHMAP | ||
| "Wenn dein Computer mit dem du den Router einrichtes am Internet angeschlossen ist, " |
There was a problem hiding this comment.
"den Router einrichtest eine Internetverbindung hat,"
| "die Koordinaten händisch zu hinterlegen." | ||
| #ifdef WITHMAP | ||
| "Wenn dein Computer mit dem du den Router einrichtes am Internet angeschlossen ist, " | ||
| "hast du die Möglichkeit auf der unten angezeigten Karte an die stelle zu klicken wo " |
There was a problem hiding this comment.
"Stelle" Großschreibung
"zu klicken wo" -> "zu klicken, an der"
| #ifdef WITHMAP | ||
| "Wenn dein Computer mit dem du den Router einrichtes am Internet angeschlossen ist, " | ||
| "hast du die Möglichkeit auf der unten angezeigten Karte an die stelle zu klicken wo " | ||
| "der Router Stehen wird." |
| "hast du die Möglichkeit auf der unten angezeigten Karte an die stelle zu klicken wo " | ||
| "der Router Stehen wird." | ||
| #endif | ||
| "Bitte berücksichtige das, das setzen einer Position die Netzwerk Qualität verbessern kann." |
There was a problem hiding this comment.
"berücksichtige, dass das Setzen"
"Netzwerkqualität"
|
Do I still have to include this block in my I built your branch https://github.com/2tata/gluon/ with GLUON_BRANCH: gluon-config-mode-geo-location, but there is no map visible (without the extra block in site.conf) Do I have to include more than just the package I think you should add a site in the docs/ section or a README to the package to explain what there is to do to include this package. |
|
On 07/20/18 17:00, Ruben Barkow wrote:
Do I still have to include this block in my `site.conf` then?
config_mode = {
geo_location = {
map_lon = 52.951947558,
map_lat = 7.844238281,
olurl = 'http://osm.ffnw.de/.static/ol/OpenLayers.js'
show_altitude = true,
},
},
This site.conf section is just optional,
Its replace the default position of the map centre (default: lat:0.0 & lon:0.0)
and the openlayers URL, if some one would like to insert its one map (default:
"http://dev.openlayers.org/OpenLayers.js")
I built your branch https://github.com/2tata/gluon/ with GLUON_BRANCH: gluon-config-mode-geo-location, but there is no map visible (without the extra block in site.conf)
Do I have to include more than just the package `gluon-config-mode-geo-location` in my `site.mk`?
The package `gluon-config-mode-geo-location` will be still available. If you
want to have the map inside you need to include
`gluon-config-mode-geo-location-with-map`.
I think you should add a site in the docs/ section or a README to the package to explain what there is to do to include this package.
I will add some doc during the day.
cheers
Tarek
|
only that one? or do I have to add besides:
there is a comma missing here |
|
On 07/20/18 18:13, Ruben Barkow wrote:
> If you want to have the map inside you need to include
`gluon-config-mode-geo-location-with-map`.
only that one? or do I have to add `gluon-config-mode-geo-location` too?
No, both packages are in conflict only one of both is possible.
cheers
Tarek
|
|
Now, there is some doc available. |
| gluon-config-mode-geo-location-with-map | ||
| --------------------------------------- | ||
|
|
||
| The package ``gluon-config-mode-geo-location-with-map`` will additionally and a |
| --------------------------------------- | ||
|
|
||
| The package ``gluon-config-mode-geo-location-with-map`` will additionally and a | ||
| map where users can pick a position. This map will only shown if the users |
There was a problem hiding this comment.
"only shown" -> "only be shown"
|
|
||
| The package ``gluon-config-mode-geo-location-with-map`` will additionally and a | ||
| map where users can pick a position. This map will only shown if the users | ||
| computer will have an internet connection. |
| site.conf | ||
| ^^^^^^^^^ | ||
|
|
||
| This option is valid for both typ of package. |
There was a problem hiding this comment.
"typ of package" -> "package variants" <-- not sure though about the meaning
There was a problem hiding this comment.
Your version sounds better.
| - represents the default latitude center of the location picker map. | ||
| - defaults to ``0.0`` | ||
|
|
||
| The above 2 options will usually shown on a factory flashed Router. If a node |
| - defaults to ``0.0`` | ||
|
|
||
| The above 2 options will usually shown on a factory flashed Router. If a node | ||
| is reenter the config mode the maps center will be on the last defined |
There was a problem hiding this comment.
"is reenter" -> "re-enters" --> not 100% sure though
"the maps" -> "the map's" (genitiv s in english)
"will be" -> "is" (not sure though)
| local pkg_i18n = i18n 'gluon-config-mode-geo-location' | ||
| local site_i18n = i18n 'gluon-site' | ||
|
|
||
| local site = require 'gluon.site' |
There was a problem hiding this comment.
check indentation in this file please, this line and the lines before have a different indentation
There was a problem hiding this comment.
Oh yes, indeed ... wrong configuration in vim..
|
|
||
| #endif | ||
|
|
||
| -- show altitude if site.json retruns true or nil and uci an value. Dont show if site.json retruns false or uci nil! |
There was a problem hiding this comment.
one space too mach indentation.
"retruns" -> "returns"
"Dont" -> "Don't"
|
|
||
| #ifdef WITHMAP | ||
|
|
||
| 'If your PC is connected to the internet you can also click on the map displayed below. ' .. |
| "If you want the location of your node to be displayed on the map, you can " | ||
| "enter its coordinates here. " | ||
| #ifdef WITHMAP | ||
| "If your PC is connected to the internet you can also click on the map displayed below. " |
There was a problem hiding this comment.
"can" -> "may" (same as before)
| #endif | ||
| "Please keep in mind setting a location can also enhance the network quality." | ||
| msgstr "" | ||
| "Um Deinen Router auf der Karte anzeigen zu können, benötigen wir seine " |
There was a problem hiding this comment.
please search for "Deinen" and "Du" and make it all lowercase (if it's not the beginning of the sentence)
The uppercase version isn't valid anymore and hasn't been for many years - afaik.
| "Plural-Forms: nplurals=2; plural=(n != 1);\n" | ||
|
|
||
| msgid "" | ||
| "If you want the location of your node to be displayed on the map, you can " |
There was a problem hiding this comment.
I think you just mean the second "you can"
There was a problem hiding this comment.
"If you want the location of your node to be displayed on the map, you may "
and this sencence is there 2 times.
There was a problem hiding this comment.
I think in the first sentence should be you can and in the second you may to have a stronger suggestion on the map.
| "If you want the location of your node to be displayed on the map, you can " | ||
| "enter its coordinates here. " | ||
| #ifdef WITHMAP | ||
| "If your PC is connected to the internet you can also click on the map displayed below. " |
| msgstr "Content-Type: text/plain; charset=UTF-8" | ||
|
|
||
| msgid "" | ||
| "If you want the location of your node to be displayed on the map, you can " |
There was a problem hiding this comment.
see my above question.
…lock the formular
2tata
force-pushed
the
gluon-config-mode-geo-location-with-map
branch
from
480594f
to
1b07343
Compare
Fixed in 1b07343
What do you mean? You have the option to set your own URL for your layers.
Fixed in 1b07343 . All external URLs are now with SSL.
How, do you know that? |
|
On 08/05/18 00:54, Ruben Barkow wrote:
rubo77 commented on this pull request.
+msgid "Static location"
+msgstr "Manuelle Position"
"manual position" translates to "Handstellung" :)
even if not: as I said, the word "manual" implements that there is a "not-manual" way also, but tif there isn't it is just strange to emphasize "manual".
I think "select position on map" suits better to all cases
I would like to avoid mentoring the map, because this is just an additional
Feature. So how about "Position" instead of "Manuelle Position" or just like the
english one "Statische Position"?
I prefer the last option "Statische Position".
cheers
Tarek
|
|
Mal auf Deutsch: Irgendwie ist das immer noch nicht so richtig logisch. "Disabled" Ist auch Quatsch, wenn man dann trotzdem sagen kann: "Share your location".
Vielleicht finden wir noch eine Formulierung, die auf alles passt. |
|
On 08/07/18 01:13, Ruben Barkow wrote:
Mal auf Deutsch:
Irgendwie ist das immer noch nicht so richtig logisch. "Disabled" Ist auch Quatsch, wenn man dann trotzdem sagen kann: "Share your location".
Bei "Disabled" kann man "Share your location" auch nicht mehr auswählen und es
wird auch nicht angezeigt.
- Was passiert denn genau bei den beiden Optionen jeweils?
"Static Position" Zeigt lat/lon/altitude Felder zur manuellen Eingabe von
Koordinaten sowie die Option das diese veröffentlicht werden, z.b. für eine map.
als Feature könnte eine Map sichtbar sein die dem User hilft eine Position zu
setzen.
"Disabled" Deaktiviert und Löscht (falls vorhanden) die Hinterlegen
Informationen. (Sprich keine Angabe von Koordinaten).
- Welche 3. Option ist geplant?
3. wären "Dynamic Position" der geolocator mit rein dynamischer Position, sowie
4. "Dynamic & Static Position" Dies erlaubt das feste setzen einer Position
gleichzeitig aber auch das scannen und übertragen von umliegenden WLANs an das
z.Z. noch openwifi.su backend.
5. Mögliche Variante wäre die Berechnung der eigenen Position aus über respondd
mitgeteilten Koordinaten umliegender direkter Nachbarn, also eine Art cooplocator.
Die o.g. Möglichkeiten sind nur Ideen und sind für den MR nicht vorgesehen
sondern sollen künftig in ähnlicher Form folgen.
- Was ist die Alternative zur Karte?
Position über die Tastatur eintippen?
cheers
Tarek
|
|
Selectbox drop-downs sind immer unübersichtlich! Da das Label ja schon "Geo-Position" ist, also am besten einfach nur eine Checkbox:
Die 3. Option kann man so später in einem anderen Modul hinzufügen:
und 4. noch später:
|
|
If there is no 3rd module, the whole first option must be hidden completely, or what sense is there to add coordinates into the node if you don't want to share it? |
|
@rubo77 you may add these coordinates so people with remote access to the node can check its location. so there is a use case for adding coordinates without sharing them. |
|
That sounds not like a use case (who would want that and why? if you want to give the coordinates to womeone with root access to your node, you could simply put a text file on the console). We should keep the config mode main page really simple, that is the main strength of Gluon compared to other firmwares. So one checkbox to enter coordinates is enough. If there later is an extra package, that makes sense to enter coordinates but keep them secret, then we should add the suggested extra checkbox, but only then. |
|
@rubo77 sure, this PR doesn't have to introduce this if it isn't possible at the moment. i just wanted to add that you and me not having the use case doesn't mean there isn't any. |
|
I've only had a short look at the code so far, but I don't like using the preprocessor to conditionalize code at all. I would like to clean this up a bit before it is merged (I might take care of it this weekend). |
| uci:set("gluon-node-info", location, "altitude", altitude.data) | ||
| else | ||
| uci:delete("gluon-node-info", location, "altitude") | ||
| end |
There was a problem hiding this comment.
This is cargo-culting from luci.model.uci. With simple-uci, uci:set(p, s, o, nil) is equivalent to uci:delete(p, s, o), so you usually don't need to check where a value is set before passing it to uci:set.
| uci:delete("gluon-node-info", location, "altitude") | ||
| uci:delete("gluon-node-info", location, "latitude") | ||
| uci:delete("gluon-node-info", location, "longitude") | ||
| uci:set("gluon-node-info", location, "share_location", 0) |
There was a problem hiding this comment.
Don't use numbers where a boolean is semantically saner (this is another thing you could not do with luci.model.uci).
|
I've finished my cleanup work: #1515 |
d6fd2c33b luci-app-travelmate: update Japanese translation 0e5d670d7 luci-app-travelmate: sync translations 8a199c3e1 luci-app-travelmate: sync with release 1.1.3 bf49505ea luci-base: properly handle undefined IPv6 local-address information 852ec6e28 luci-base, luci-mod-admin-full: store backup vars in luci configuration 68bdfc029 luci-app-adblock: fix the msgid to keep the translations dcb041e39 luci-app-mwan3: remove css from javascript in overview_status_interface 5362f6970 luci-app-mwan3: Update header name in status pages a73687022 luci-app-mwan3: remove duplicated code section deba02e09 luci-app-mwan3: fix source styling 1ff26f380 luci-app-mwan3: remove css styling from status_interface page a82e588c0 luci-app-mwan3: remove css styling from overview_status_interface page 3c86c6562 luci-app-mwan3: fix stack trace if no tacking_ip is set on interface add d098a64e2 community-profiles: potsdam - changed SSID 391a7bfc0 luci-app-travelmate: sync with release 1.1.2 b1c5a4c07 luci-app-adblock: fix frontend typo 6ffb7056f luci-app-travelmate: sync with release 1.1.0 461df8b0d luci-base: do not assume a fixed host address in delegated prefix (freifunk-gluon#1484) de4f1c904 luci-base: rework client side IP validation types and support "hostid" type 3e1e4d5eb luci-base: fix Lua-side ip6hostid() datatype validation c21d22c01 i18n-ru: fixed russian translation 66b93ab78 luci-app-wman3: show info if mwan3 is not global enabled 4b0347d49 luci-app-mwan3: update look and feel interface status page 83cec9277 luci-app-mwan3: do not show mwan3 status in overview if not enabled 0c3bd5c53 luci-app-mwan3: update uci track entry to use /etc/init.d/mwan3 reload 325f6d413 luci-app-nlbwmon: update Japanese translation d06865000 luci-app-uhttpd: update Japanese translation 770033dba luci-app-shadowsocks-libev: remove deprecated option disable_sni 28aafed47 luci-app-shadowsocks-libev: add no_delay to names_options_common e6cfe35ba luci-mod-admin-full: prevent unknown sysctl key warnings on status page 71230d2b7 luci-app-adblock: fix XHMTL Markup eae2f37b6 timezone data: update to 2018c 69932a934 luci-app-adblock: update Japanese translation 7d2326291 luci-app-adblock: sync translations 47e0990ea luci-app-adblock: sync with adblock 3.5.0 43589ae4d Fix freifunk-gluon#1609: luci-proto-wireguard placeholder wrongfully suggests default port 4567512bd i18n-ru: fixed and updated russian translation f11b0c000 luci-app-travelmate fixed 7133c5101 i18n-ru: fixed and updated russian translation 1cd12c449 luci.app-ddns: Update to 2.4.9-1 d74ff878e i18n-ru: fixed and updated russian translation 870c176ca luci-app-simple-adblock: bugfix for service start/stop 61cf34428 fixed dynapoint.lua, there was no translation 1fd24f3fd luci-app-vpnbypass: better service start/stop dbdb91e96 luci-app-firewall: zh-cn: Update Simplified Chinese translation 582ce1c69 fixed dynapoint.lua, there was no translation c38117d2a luci-app-wman3: add license header to all files 72191b7e2 luci-app-mwan3: fix diagnostics dropdown menu entry 562a3e5b7 luci-app-mwan3: fix assert if route could not select by ubus 3467df984 i18n-ru: fixed simple-adblock strin&rus translation 1295b6ca3 i18n-ru: fixed russian translation 57fe3e839 luci-app-advanced-reboot: fixed bug on devices/board names with dashes in them 3a25e1503 i18n-ru: fixed 194e42913 luci-app-voice-core and -diag: remove from repo 7edc6f03f freifunk-p2pblock / luci-app-p2pblock: remove from repo 6a8ee2620 luci-app-diag-devinfo: remove from repo 5cc9df99e luci-app-ushare: remove from repo d0441ee87 luci-app-radvd: remove from the repo b2754db22 luci-app-pbx(-voicemail): remove from repo 8a929a955 i18n-sync base.po changes bb87aac85 i18n-ru: Edits on the general pattern of Russian translation 706225070 luci-mod-admin-full: fix typos on dhcp page dc71ae010 i18n-sync changes mwan3
This MR is a split of #1211
Its add an osm map into the config-mode (in case the clients computer has an internet connection).
Issue #58
export zoom level to site.conf.
check translation.
Signed-off-by: Jan-Tarek Butt tarek@ring0.de