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

Initial Node List #6

Merged
merged 12 commits into from
Aug 22, 2017
Merged

Initial Node List #6

merged 12 commits into from
Aug 22, 2017

Conversation

darkdrgn2k
Copy link
Contributor

Initial node list for use on the tomesh.net site tomeshnet/tomesh.net#151

darkdrgn2k added 2 commits July 22, 2017 17:16
@benhylau
Copy link
Member

What is the source of this list?

@darkdrgn2k
Copy link
Contributor Author

@benhylau The existing temporary map (map.tomesh.net) which was original primary derived from @Pedro-on-a-bike and a handfull of other manual nodes. Its the same list that is currently powering tomesh.net dev branch's map.

nodeList.json Outdated
"cardinalDirection":"South",
"floor":"1",
"meshHardware":"Unknown",
"IPV6Address":"h.groundupworks.com",
Copy link
Member

Choose a reason for hiding this comment

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

Should we require this field be address only? fcaa:5785:a537:90db:6513:bba9:87a0:12a7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, what happens when the addresses changes though?

Honesty the schema has not been finalized yet i just wanted to start somewhere.

Copy link
Member

Choose a reason for hiding this comment

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

A node having a copy of this list should be able to reach every node without internet connection, but if some of the node addresses are domain names we would need DNS, which currently does not exist in-mesh. I am okay merging as is, since it's trivial to fix this later.

"latitude":43.63518200000000,
"longitude":-79.40201470000000,
"onlineStatus":"Online",
"cardinalDirection":"South",
Copy link
Member

Choose a reason for hiding this comment

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

Should these be [ N E S W NE NW SE SW ] for better resolution?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, actually I think those exist already, nevermind: #5

@dcwalk
Copy link
Contributor

dcwalk commented Aug 15, 2017

Our initial schema draft elsewhere #2 doesn't have all these field specified, (and addresses your questions @benhylau)

I'm in favour of merging this in as is, because as @darkdrgn2k says it is currently powering the map but doesn't live in a tomesh repo 😨 . Then we should get that schema reviewed and merged in (#2 should be good to go), and update this list to reflect that.

@@ -0,0 +1,900 @@
[
Copy link
Member

Choose a reason for hiding this comment

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

Should this be an object instead of an array? Referencing the schema

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code i got from Pedro seems to reference it as an array, so the site is coded to look for an array. I would have to go back and change the code to treat it as an object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran the Schema and the Node List via http://www.jsonschemavalidator.net/ and it checks out. In fact Javascript and above site errors out parsing { instrad of [

Copy link
Member

Choose a reason for hiding this comment

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

We'd need to add keys to the node objects for to it validate. I'd be okay with keeping it as is for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

These comments have been moved to #8 for tracking!

nodeList.json Outdated
"onlineStatus":"Offline",
"cardinalDirection":"North West",
"floor":23,
"meshHardware":"None",
Copy link
Member

Choose a reason for hiding this comment

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

For meshHardware and IPV6Addres: We alternate between None and Unknown. I wonder if it would be better to have them be empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently there is a difference between the two

None means there simply is non
Unknown means that it exists, we just don't know what it is

For example there IS hardware in willow-dale mesh but there is NO hardware in most of the possible spots.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need "None" then? I'm assuming that is the case for proposed? (The field isn't required IIRC and isn't in the initial schema)

darkdrgn2k added 3 commits August 15, 2017 17:08
Changed structure from ARRAY to OBJECT
Removed some custom fields
nodeList.json Outdated
"lastPing":"-1",
"rtt":"0"
"status":"active",
"dateAdded":"2017-18-15T00:00:00.0Z"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be 2017-08-15?

nodeList.json Outdated
"onlineStatus":"Offline",
"cardinalDirection":"North West",
"floor":23,
"meshHardware":"None",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need "None" then? I'm assuming that is the case for proposed? (The field isn't required IIRC and isn't in the initial schema)

nodeList.json Outdated
"lastPing":"-1",
"rtt":"0"
"status":"active",
"dateAdded":"2017-18-15T00:00:00.0Z"
},
{
"name":"Willowdale Mesh Node",
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't aware that this had been added prior to confirming. Can we identify what nodes in the list are like that?
I'd prefer they not be added in unless confirmed/added by the individual

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already reached out to confirm, removed until then.

nodeList.json Outdated
"cardinalDirection":"Omni",
"floor":"0",
"status":"active",
"dateAdded":"2017-15-18T00:00:00.0Z"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the new commit still carried forward an issue with the date
2017-15-18T00:00:00.0Z
should be 2017-08-15T00:00:00.0Z

@dcwalk
Copy link
Contributor

dcwalk commented Aug 16, 2017

It looks like we are almost ready to merge in this first pass?

My one thought is we should specify a license for the data, would be nice to add in on this PR if possible before it lives in the repo!

@dcwalk
Copy link
Contributor

dcwalk commented Aug 22, 2017

Great, glad we are all happy with getting something in now! I'm gonna merge 🎉 @darkdrgn2k for pushing this through. Lets address Travis CI, next iteration of spec etc... down the road, but now we have everything to host the proposed first version of the map.

@dcwalk dcwalk merged commit 4836f3c into tomeshnet:master Aug 22, 2017
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.

None yet

4 participants