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

Added script to post your location automatically on the allsky map #1073

Merged
merged 15 commits into from
Mar 19, 2022

Conversation

thomasjacquin
Copy link
Collaborator

@thomasjacquin thomasjacquin commented Mar 13, 2022

I do get a lot of requests to add users to the allsky map (http://www.thomasjacquin.com/allsky-map/).

It currently is a manual process. I need to add a pin on a Google Map with the user details (location, lat, long, website url, image url).
The data is hardcoded in the index file. It loads all cameras at once (along with the image) and it slows down the load of the page.

Here's the current format:

map.addMarker({
    lat: 60.771923,
    lng: -135.139694,
    title: 'Whitehorse',
    infoWindow: {
        content: '<h1>Allsky</h1><p>Owner: Thomas Jacquin<br>Whitehorse, YT<br>Canada</p><p><a href="http://thomasjacquin.com/allsky" target="_blank"><img src="http://thomasjacquin.com/allsky/fireweed-resize.jpg" width=200 decoding="async"></a></p>'
    }
});

This PR adds a script to post the camera details once a day (endOfNight script but open to other suggestions). If a user changes details about their setup, it gets propagated to the website automatically.
Users can choose to display their website url or not. If no website is provided, only a pin gets displayed.

The server side code filters requests (insert vs update) based on IP address as a unique ID.

Let me know what you think. I'm open to changes. The goal here is to have more cameras (and camera setup info) on the map without any action from the user or myself.

New map for testing: http://www.thomasjacquin.com/allsky-map/index-new.php

cc @EricClaeys I've mentioned this to you last month. I just got back home so I can work on this now.

Copy link
Collaborator

@EricClaeys EricClaeys left a comment

Choose a reason for hiding this comment

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

@thomasjacquin Welcome back to Allsky. I'm glad you remember how to do a PR :-)
Several people agreed to help test this - see the Discussions link for their names. I will coordinate with them.

I will look at this in more detail, but some initial thoughts:

  • thanks for creating the PR. I haven't had a chance.
  • we plan to merge all the allsky config files into one .json file that will support multiple cameras, so it might be best to not introduce another config file at this point. Some of the info in postToMap.sh is already in the settings*.json files, and some can be automatically determined (as is done with config.js in the allsky-website package), so I suggest putting all the data in the settings* file, including the POST_TO_MAP boolean. We can create a new group in the file so all the settings appear together in the WebUI. When we get to one json config file it will be managed via the WebUI, so adding the postToMap data there now seems like a natural fit.
  • How will IP address work for people like me whose Pi is behind a firewall and has a 192.168.* address? We may get a lot of duplicate IPs. Should we use the IP and, for example, owner's name as the unique key?
  • After we last talked I thought of some other information that would be useful to have sent to the server, like Allsky version, whether or not they have the WebUI and website installed, and some other info I can't remember. The info doesn't need to appear on the map, but assuming us developers have a way to access the info, it might help us decide what features to work on, etc. We of couse need to make sure users know what is being sent to the server to eliminate privacy concerns.
  • Uploading every day isn’t needed, is it? We had talked about the data expiring in a month. If we can think of a way to have the info uploaded once a week that would be easier on your server, especially if we can spread out which day of the week, possibly by using the last octet of the IP address and taking the remainder of dividing by 7.
  • We are trying to keep all scripts in allsky/scripts so I suggest moving this script there. Invoking it from endOfNight.sh would then be "${ALLSKY_SCRIPTS}/postToMap.sh"
  • what is the .gitignore file for? It has some obsolete files in it (e.g., saveImageDay.sh) and existing files.

@thomasjacquin
Copy link
Collaborator Author

Thanks @EricClaeys, things have been crazy busy with work this year but I'm trying to balance it out better. I probably won't have a huge amount of time to spend on the Allsky project but I'll try to stay up to date with the latest changes. There are a few small enhancements I've had in mind for a while so I'll make sure to take the time to submit more PRs.

  • I'll keep an eye on the merge of the config files.
  • Regarding the unique key, we can do a combination, there's no issue with that. We just need to pick 2 (or more) fields that can't be null.
  • I can create a password protected admin page for developers to view that extra information. The users would have a choice to send that info or not (similar to those checkboxes "I agree to send anonymous data to help make our product better...").
  • Uploading everyday isn't needed, we just need an easy way to upload the changes. Most of the time, they won't change but if someone changes their lens or observatory name, it would be nice if it doesn't take a week to reflect the changes. Maybe we call that script automatically when some one of those fields has changed.
  • That script can move anywhere. I wasn't sure what the preference was.
  • .gitignore lists files that you don't want to track in the repository. For example, you want to keep track of the .json.repo files but not thee .json because they are likely to change a lot. When you run git status it lists the files that are not tracked. Obsolete files can be taken out though.

@EricClaeys
Copy link
Collaborator

@thomasjacquin
The "Projects" link at the top of the page list most of the enhancements we've been considering, so you may want to take a look at it to see if your enhancements are there.

  • I think it's safe to say the latitude and longitude must be present, or else we can't put a pin on the map. The other fields, while strongly suggested, don't strictly have to be there, do they, although we should force IP to have something? A Would it make sense to use latitude, longitude, and IP for a unique key?
  • The IP address on the website has N or S and E or W, but in the WebUI has just a decimal number. Can the server program accept either? It would actually be nice if the WebUI accepted either, but then the programs that call sunwait would need to convert to decimal numbers.
  • When we talked a while ago about this, the server program was going to return status. I forget the exact status, but basically an "OK" or an error message. The postMapData.sh program will need to parse that output.
  • Assuming postMapData.sh is changed to get the date from various sources, it would be easy to change the WebUI to call it when the user updated settings. Doing something similar is actually on my to-do list, for example, changing the latitude, longitude, or angle should call postData.sh. In the meantime, postMapData.sh could be called every other day, for example, if the last digit of the IP was even, it would be called on even numbered days. That would at least half the number of requests your server sees every day.
  • Is there a way to have a file in the GitHub page but NOT include it when someone does a git clone?

@thomasjacquin
Copy link
Collaborator Author

thomasjacquin commented Mar 17, 2022

@EricClaeys

  • My reasoning here is to have a unique ID per camera. I'm trying to think of various use cases. For example, a user refines his latitude from 47.05 to 47.052. The latitude has now changed but we don't want to insert a new pin on the map.
    Maybe the best would be to use the mac address of the interface used by the camera as the primary key. All devices should have one in theory and even though it can be changed manually, it is not likely to happen.

  • Google maps uses decimals for lat and long. Sunwait does too. I think the NSEW labels are nice but they should only be used on the frontend. The WebUI could convert between decimal and letter-based on the page but it would still save to json as decimal
    (edit: I forgot we're saving them with the NSEW label in json so I'll give more thoughts to this).

  • The server side (postToMap.php) will return different messages based on the action it performed. I am thinking inserted, updated, removed.

  • I like the idea with the last digit of the IP.

  • I think git clone retrieves everything on the repo. That's why I put in place the .repo files originally. During installation, they are copied to .json and .sh but these are added to .gitignore so we don't post our settings or ftp credentials publicly on the repo.

@thomasjacquin
Copy link
Collaborator Author

I have added a PR for the allsky-portal thomasjacquin/allsky-portal#96
I believe these 2 PRs (allsky and allsky-portal) cover the basic needs and could be tested by volunteers.

Side note: How does one call a script manually with a dynamic source path?
Example: source "${ALLSKY_CONFIG}/config.sh". ALLSKY_CONFIG isn't define when running ./postToMap.sh

Side note 2: I am unsure why git included the allsky_common source files in this PR. Mine should be discarded in case of merge conflict.

Copy link
Collaborator

@EricClaeys EricClaeys 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.
The "Added mac address as unique ID" commit changed the src/allsky_common.cpp and src/include/allsky_common.h files - it looks like every line was changed, but I can't tell what the changes are. Do you know?

I assume you've tested this?

@EricClaeys
Copy link
Collaborator

@thomasjacquin
All scripts that output anything that goes to /var/log/allsky.log should set $ME to the name of the script, then use it in all output so it's easy to determine what program put the line in the log file.
Any script designed to be called directly from the command line needs this near the top of the file (see endOfNight.sh for an example)

ME="$(basename "${BASH_ARGV0}")"

# Allow this script to be executed manually, which requires several variables to be set.
if [ -z "${ALLSKY_HOME}" ] ; then
	export ALLSKY_HOME=$(realpath $(dirname "${BASH_ARGV0}")/..)
fi

Side note: All scripts need to source "variables.sh" before sourcing "config.sh" since variables.sh defines all the $ALLSKY_* variables except ALLSKY_HOME:

source "${ALLSKY_HOME}/variables.sh"
source "${ALLSKY_CONFIG}/config.sh"

Sorry I missed those when reviewing the PR.

@EricClaeys
Copy link
Collaborator

@thomasjacquin
sunwait requires NSEW. It gives an error without them. It would be nice if sunwait accepted either way (I looked at it once but couldn't figure out how to change it), and if the Allsky website accepted either in the config.js file.

@thomasjacquin
Copy link
Collaborator Author

The changes to src/allsky_common.cpp and src/include/allsky_common.h are line endings so it's pretty safe. Probably a side effect from pulling the code on Mac and Linux.
I have disabled a few shellcheck warnings because they were preventing CI to compile. I'm not sure what good practice you guys are usually using.

@EricClaeys EricClaeys merged commit 21c50ff into master Mar 19, 2022
@EricClaeys
Copy link
Collaborator

@thomasjacquin. Andreas has been working primarily on the RPiHQ code and shellcheck and other tests. I have been making all the other checks. Chris was absent for a few months and Michael has been helping as time permits.

There are a bunch of PRs waiting for review/approval so if you have time I would appeciate you looking at them.

The shellcheck is new so we don't have any standards yet, but I have been trying to resolve them as I make other changes, plus there is a branch just for shellcheck changes.

@thomasjacquin thomasjacquin deleted the post-camera-top-allsky-map branch March 20, 2022 04:33
@thomasjacquin
Copy link
Collaborator Author

@EricClaeys Thanks for the info. I'll take a look at those PRs and give my input if it's in my field of expertise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants