aerosense-ai / data-gateway

Data influx for Aerosense.
https://www.aerosense.ai/
Other
3 stars 1 forks source link

Update Configuration class to support multiple nodes #70

Closed thclark closed 2 years ago

thclark commented 2 years ago

Summary

Updates the configuration object to allow for multiple nodes.

Contents (#70)

IMPORTANT: There are 2 breaking changes.

New features

Fixes

Operations

Refactoring

Testing

Chores


Upgrade instructions

πŸ’₯ Remove incorrectly parameterised default sensor name MICROPHONE_SENSOR_NAME is no longer exported This was removed because it was a misnomer (actually should have been called DEFAULT_MICROPHONE_SENSOR_NAME). It's very unlikely this will cause any onward breaking chandes, but if using that variable externally, the user should instead access `data_gateway.configuration.DEFAULT_SENSOR_NAMES[0]` which is the same thing.
πŸ’₯ Update configuration for multiple nodes per #64 Configuration values nested and separated by concerns - Users should consult docs for the new configuration format. Older `configuration.json` files will no longer work so need to be updated. - Periods are now calculated rather than being configurable, to eliminate duplication. Users should configure the relevant frequency parameters for the sensors. - The `session_data` key was converted to `session` - The ambiguous `hardware_version` key was removed and replaced with `gateway.receiver_firmware_version` and `nodes[node_id].node_firmware_version` - The `packet_key` is no longer present, instead use `Configuration().get_packet_key(node_id)`
thclark commented 2 years ago

@cortadocodes here's my proposition of what the configuration object should look like. Of course, this doesn't pass tests yet. Things to consider:

cortadocodes commented 2 years ago

We need to talk about a convention/pattern for the upgrade instructions, because they need to be there but are too verbose as-is. Perhaps we could put them into <details> sections, where the first line of the breaking change note is the summary line?

I'll make an issue and candidate PR in conventional-commits

thclark commented 2 years ago

@cortadocodes this si good to go, merge whenever you're ready

thclark commented 2 years ago

@cortadocodes no, I can't make the tests pass on this branch. The other branch is WIP on doing that, but you explicitly requested small PRs so you could see what I was doing one chunk at a time. I can either:

Which do you want?

cortadocodes commented 2 years ago

Ah I see that you can't get all the tests to pass in this branch but I think my first and possibly second bullet points fall within this PR as they're directly to do with the configuration. I expect this will stop some of the DataGateway and PacketReader tests from passing but that's for another PR as you say


  • Update tests/valid_configuration.json so the configuration tests pass

valid_configuration.json is no longer a valid configuration but can be converted to a one-node config. The configuration tests only relate to the Configuration class so I think it makes sense to update them here


  • Update usages of Configuration.from_dict to the new way of instantiating

The from_dict method of the Configuration class was removed - shouldn't we swap its usage here?