[WAIT] - Automationtest

[davidelang]

The splitaplist script splits the aplist.csv into apmac.csv (map of serial number to mac address)
and apuse.csv (the rest of the info, not counting the ipv4 network prefix)

there is an additional file roommap.csv that identifies:
what building each room prefix is in
what the ip range is for that room
what map id it would appear on

buildaplist takes these files and creates a new aplist.csv that has the correct combination of information

when editing the aplist directly we tend to change the name and the rest of the stuff stays the same
this breaks when the spare was in a different building
it also drags the rest of the per-location information from the spare

This new approach lets us change what AP is where by editing the apuser.csv file
with the mac changing automatically in the aplist.csv file
while keeping the rest of the per-location information being the same

[davidelang]

talking with Rob, do we want to eliminate aplist.csv entirely and have a checkin/deployment time script use the new files directly?

this would avoid confusion for people who see the aplist file and think it can/should be edited directly (the splitaplist script can take an aplist file and extract the other files from it, so if someone does edit aplist directly, we have a path to use the new approach, it just leads to confusion)

[davidelang]

additional commit time checks can be written to make sure there are no duplicate mac addresses or serial numbers in the apmac and apuse files

future automation can auto-assign IP addresses and channels (for a first pass to make sure we don't have everything in the room on the same channel anyway :-) )

[kylerisse]

I tend to agree @davidelang . There's no need to maintain a derivative file aplist.csv if it can be reproduced and validated at deploy time consistently, especially through the Nix process. I can help.

[sarcasticadmin]

This all seems reasonable enough, as a part of this Ill work on updating the drv so that this information actually flows to the kea config. Then we can drop the original aplist.csv all together

[owendelong]

IMNSHO:

We should only have one source of truth for any datum maintained in the repo. Any file which can be produced from other files in the repo containing the data should not be maintained in the repo and should be treated as object code built from the source code in the repo.

[davidelang]

I agree, I want the separate files to be the source of truth, I made the script to create them from the aplist.csv as a transition tool. Rob is talking about modifying the tooling that consumes aplist to instead consume the separate files. I'm in favor of this, but was not pushing this initially just to minimize the impact of changes. David Lang

…

On Thu, 23 Mar 2023, Owen DeLong wrote: IMNSHO: We should only have one source of truth for any datum maintained in the repo. Any file which can be produced from other files in the repo containing the data should not be maintained in the repo and should be treated as object code built from the source code in the repo.

[sarcasticadmin]

Rob is talking about modifying the tooling that consumes aplist to instead
consume the separate files. I'm in favor of this, but was not pushing this
initially just to minimize the impact of changes.

Yep exactly

[owendelong]

Excellent. Sounds like we all agree.

[davidelang]

see #494 and handle as part of the tooling

[owendelong]

This is still labeled wait, but has been reviewed and appears ready for merging... Is there still a reason for the WAIT designation?

[owendelong]

If the WAIT is for the retooling, I see two reasonable scenarios and I'm OK with either one, but we should pick one and act accordingly:

1. Merge this PR and do the needful with the retooling (including deprecating the split script) in a later PR
   or
2. Reset the review state on this PR and do a fresh review when those additional changes have been incorporated into it.

I'm not OK with leaving this PR waiting for additional work after being reviewed such that it can be merged without further review after more (significant) work is done on it. It's one thing to do minor corrections based on the review and then merge, but an entirely different thing to make significant additional changes without having a review required.

[sarcasticadmin]

After taking another look at here were some things that popped out to me:

1. With this PR we still dont have a single, isolated source of truth for the serial or mac address. Since aplist.csv is required to be present to regenerate a new aplist.csv since its the only place that the serial and mac address are stored. My preference would be to have this be an input to aplist.csv this way its no longer modified after we confirm the inventory of the aps. This would go a long way to actually tracking our complete list of aps
2. roommap.csv doesnt seem necessary since we can get this from https://github.com/socallinuxexpo/scale-network/blob/e857c20374465b3b1b810c70c8339a65286b4156/switch-configuration/config/vlans.d/Conference

What I think would be better would be to separate this into two files slightly differently:

1. serial,mac address - This should change pretty much never unless we update our inventory of APs. Call this aps.csv
2. name,serial,ipv4.ipv4,2.4Ghz_chan,5Ghz_chan,config_ver,map_id,map_x,map_y - The stuff that pretty much already makes up apuse.csv but is subject to change

Bonus here is that we could get away from labeling spares in the apuse.csv but keep them tracked via the aps.csv

[owendelong]

After taking another look at here were some things that popped out to me:

1. With this PR we still dont have a single, isolated source of truth for the serial or mac address. Since aplist.csv is required to be present to regenerate a new aplist.csv since its the only place that the serial and mac address are stored. My preference would be to have this be an input to aplist.csv this way its no longer modified after we confirm the inventory of the aps. This would go a long way to actually tracking our complete list of aps
2. roommap.csv doesnt seem necessary since we can get this from https://github.com/socallinuxexpo/scale-network/blob/e857c20374465b3b1b810c70c8339a65286b4156/switch-configuration/config/vlans.d/Conference

What I think would be better would be to separate this into two files slightly differently:

1. serial,mac address - This should change pretty much never unless we update our inventory of APs. Call this aps.csv
2. name,serial,ipv4.ipv4,2.4Ghz_chan,5Ghz_chan,config_ver,map_id,map_x,map_y - The stuff that pretty much already makes up apuse.csv but is subject to change

Bonus here is that we could get away from labeling spares in the apuse.csv but keep them tracked via the aps.csv

I really like this approach... David? Any thoughts?

[owendelong]

Hoping this will change the status of my previous approval.

[owendelong]

Hoping this will change the status of my previous approval.

Yay.. That worked.

[davidelang]

On Tue, 7 Nov 2023, Owen DeLong wrote: > After taking another look at here were some things that popped out to me: > > 1. With this PR we still dont have a single, isolated source of truth for the `serial or mac address`. Since aplist.csv is required to be present to regenerate a new aplist.csv since its the only place that the `serial and mac address` are stored. My preference would be to have this be an input to aplist.csv this way its no longer modified after we confirm the inventory of the aps. This would go a long way to actually tracking our complete list of aps > 2. `roommap.csv` doesnt seem necessary since we can get this from https://github.com/socallinuxexpo/scale-network/blob/e857c20374465b3b1b810c70c8339a65286b4156/switch-configuration/config/vlans.d/Conference > > What I think would be better would be to separate this into two files slightly differently: > > 1. `serial,mac address` - This should change pretty much never unless we update our inventory of APs. Call this `aps.csv` > 2. `name,serial,ipv4.ipv4,2.4Ghz_chan,5Ghz_chan,config_ver,map_id,map_x,map_y` - The stuff that pretty much already makes up `apuse.csv` but is subject to change > > Bonus here is that we could get away from labeling spares in the `apuse.csv` but keep them tracked via the `aps.csv` I really like this approach... David? Any thoughts?

sounds good to me. David Lang

[davidelang]

roommap is covering data not in switchconfig.

this is related to the idea of creating a monitoring map that shows devices overlayed on a map of the facilities. The split may not be the right one yet as the coordinates of the AP on the map are in the apuse file.

a better thing for this may be to have the roommap also have some coordinates to define the rectangle of the room and then have the coordinates in the apuse file be relative coordinates

note that we are not currently generating these maps, so it doesn't really matter yet.

[owendelong]

I think that having a room map file that contains coordinate sets defining the polygon comprising the room boundaries in absolute terms relative to some defined origin and then having the AP coordinates in the apuse file relative to the first coordinate defined for the room makes a great deal of sense and allows us to stick to the idea of “one source of truth”. I would argue that not all rooms are rectangular (though I suppose all of our current venue rooms technically are), so I do argue for the idea of a variable length list of coordinates that are assumed to define a closed polygon such that: “Room” : { “Name”: “BallOddShape”, “Coordinates”: { [ 5, 20 ], [ 8, 34 ], [ 20, 20 ], [ 25, 6 ] [ 5, 6 ], } } Would define a room starting at the northwest corner proceeding east along the north wall (triangular), then south along the east wall, west along the south wall, with the west wall (inferred) to be a north south line from 5,6 to 5,20. I believe that the above is valid JSON syntax to represent same. That’s just a proposal, I’m not wedded to format or mechanism, but it at least gives us a straw man to throw eggs at or light on fire if we decide we don’t like it. Owen

…

On Nov 8, 2023, at 02:16, David Lang ***@***.***> wrote: roommap is covering data not in switchconfig. this is related to the idea of creating a monitoring map that shows devices overlayed on a map of the facilities. The split may not be the right one yet as the coordinates of the AP on the map are in the apuse file. a better thing for this may be to have the roommap also have some coordinates to define the rectangle of the room and then have the coordinates in the apuse file be relative coordinates note that we are not currently generating these maps, so it doesn't really matter yet. — Reply to this email directly, view it on GitHub <#585 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAK6GTTVAW6GDQGIHTIWUODYDNLWVAVCNFSM6AAAAAAVX2TEDWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBRGUZDKOJVGI>. You are receiving this because you commented.

[tweak42]

In consultation with David, I created a database schema that breaks down the data fields into three tables / files.

See the pretty graphic here:
https://dbdiagram.io/d/Scale-Access-Point-table-reference-655930ae3be14957874478d7

Here's a copy paste of the raw text, including comments.

Table AP_physical {
  serial_num varchar [primary key]
  mac_address varchar
  problem_flag varchar // text field for problems
}

Table Rooms_facts {
  room_name varchar [primary key]
  mapID integer // see site map
  bottom_left_px integer  // screen pixel dimensions
  top_right_px integer
  dim_x_ft integer // physical room dimension
  dim_y_ft integer
  }

Table AP_locations {
  AP_location varchar [primary key]
  room_name varchar // implied field, derived from AP_loc, split AP_loc by - and use the first field as RM_name
  serial_num integer
  IP_address  varchar
  channel_24 integer
  channel_50 integer
  switch_config varchar
  x_ft integer // coords inside of room
  y_ft integer
}

Ref: AP_physical.serial_num - AP_locations.serial_num
Ref: Rooms_facts.room_name < AP_locations.room_name

[owendelong]

In addition to this, we need to set some standards for how we measure things. I’d like to see us agree on the idea of a datum point at or southwest of the site bounding box as origin for all map coordinates. I would like to see us standardize the southwest corner of the room bounding box as origin for all room coordinates. I have a slight preference for true north (it wanders less and is used for most maps) vs. magnetic north, but could be persuaded to magnetic if there is good cause. By bounding box, I mean the smallest rectangle with sides running north-south and east-west which can contain the entire room. For a rectangular room with walls running north-south and east-west, this would be identical to the room. For rooms which are not north-south/east-west oriented, or which are not rectangular, the bounding box will be larger than the room. Measurements taken of the room and the position of the APs will likely need to use one corner of the room and then we will have to compute the offset from that point to the bounding box corner for database purposes.

On Nov 18, 2023, at 16:15, tweak42 ***@***.***> wrote: In consultation with David, I created a database schema that breaks down the data fields into three tables / files. https://dbdiagram.io/d/Scale-Access-Point-table-reference-655930ae3be14957874478d7 Here's a copy paste of the raw text, including comments. Table AP_physical { serial_num varchar [primary key] mac_address varchar problem_flag varchar // text field for problems } Table Rooms_facts { room_name varchar [primary key] mapID integer // see site map bottom_left_px integer // screen pixel dimensions

I would rather see this in a more useful unit of measure… Also, this should be a pair of integers (X and Y). There should be an additional table (IMHO) for Map Metadata which would provide some sufficiently detailed real world coordinates for the map extents (e.g. bottom left and top right D/M/S.SSSS, or we could have a site origin datum and have all maps extents measured in distance units from that datum, etc. I don’t care too much how we skin the particular cat, but I do want to be able to support visualization against multiple maps from the same room data, which would require room coordinate data to be in real world units relative to some datum and map pixels to be a computed value from that.

top_right_px integer dim_x_ft integer // physical room dimension dim_y_ft integer

dim_x_ft should be a float. Ft is not sufficient resolution IMHO. I would rather see us use dim_x_mm and dim_y_mm, personally. We can always display that in whatever units we choose, but storing as mm seems a more rational approach in my mind.

} Table AP_locations { AP_location varchar [primary key] room_name varchar // implied field, derived from AP_loc, split AP_loc by - and use the first field as RM_name serial_num integer IP_address varchar channel_24 integer channel_50 integer switch_config varchar x_ft integer // coords inside of room y_ft integer

Similar comment here… I would rather see x_mm and y_mm as offset from room origin.

} Ref: AP_physical.serial_num - AP_locations.serial_num Ref: Rooms_facts.room_name < AP_locations.room_name

Overall, I’m very happy to see this effort making progress and I think the ideas being pushed forward are good. Owen

[sarcasticadmin]

@davidelang arent we missing the csv with the serials and macs?

[davidelang]

These scripts create that file from the aplist when run, since the aplist in this PR isn't correct, we need to first add these scripts, then run the split to split the current aplist into the different files. Then we need to change aplist from the source of truth (and tracked in git) to be generated by the build script and track the other files in git. David Lang

…

On Mon, 27 Nov 2023, Robert James Hernandez wrote: @sarcasticadmin commented on this pull request. > @@ -0,0 +1,36 @@ +name,map_id,ll_px_x,ll_px_y,ur_px_x,ur_px_y,x_ft,y_ft @davidelang arent we missing the csv with the serials and macs?

[owendelong]

Closing with prejudice, will not be implemented.