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

[DA] Add per-device addressing (DA) to WAN topic/channel decoding strategy #136

Merged
merged 4 commits into from
Jun 7, 2023

Conversation

amotl
Copy link
Member

@amotl amotl commented May 25, 2023

About

This patch implements the proposal outlined at GH-133, adding a generic per-device topic addressing scheme, not exclusively but also in order to bring in multi-tenant TTN support, see GH-8.

Documentation

... is still missing. But reading the code, specifically the test cases and inline comments, will give you an idea.

Details

  • Per-device addressing and topic/channel decoding strategies. Valid examples are:
    # Addressing a device.
    mqttkit-1/device/123e4567-e89b-12d3-a456-426614174000
    
    # Addressing a channel.
    mqttkit-1/channel/network-gateway-node
    

Backlog

  • Channel isolation / authentication, see [Proposal] Add a generic device-based addressing scheme for "WAN" networks #133 (comment).
    => Resolved with 0e60d11.
  • Maybe also implement at LanStrategy, not only at WanBusStrategy?
    => LanStrategy will be dissolved in favor of SensorWAN direct device addressing.
  • What about the data export path?
  • As a followup, what about per-device downlink messages?
  • Documentation, both for the generic addressing scheme, for example using UUIDs as device identifiers, and the dashed topic addressing scheme, relating to multi-tenant TTN acquisition.
    => Documentation for multi-tenant TTN acquisition will be added with a subsequent patch.

This aims towards bringing in multi-tenant channel bundle endpoints,
for example suitable to accepting TTS/TTN payloads, in order to run
a network of multiple devices on a single TTN application conveniently.

The feature is called "device addressing" (DA).

Examples:

- mqttkit-1/d/123e4567-e89b-12d3-a456-426614174000
- mqttkit-1/dt/network-gateway-node

It works with both MQTT and HTTP protocols.
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.23 🎉

Comparison is base (2a0b51c) 53.02% compared to head (d2151b6) 53.26%.

❗ Current head d2151b6 differs from pull request most recent head 1fa3ca5. Consider uploading reports for the commit 1fa3ca5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #136      +/-   ##
==========================================
+ Coverage   53.02%   53.26%   +0.23%     
==========================================
  Files         100      100              
  Lines        5267     5291      +24     
==========================================
+ Hits         2793     2818      +25     
+ Misses       2474     2473       -1     
Flag Coverage Δ
unittests 53.26% <100.00%> (+0.23%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
kotori/daq/strategy/wan.py 100.00% <100.00%> (+3.44%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment on lines 122 to 157
@pytest.mark.strategy
def test_wan_strategy_device_dashed_topo_too_many_components_redundant_realm():
"""
Verify the per-device WAN topology decoding, using a dashed device identifier with too many components.

This is an edge case where the first addressing component actually equals the realm.
Strictly, it is a misconfiguration, but we pretend to be smart, and ignore that,
effectively not using the redundant information, and ignoring the `myrealm-` prefix
within the device identifier slot altogether.

Cheers, @thiasB.
"""
strategy = WanBusStrategy()
topology = strategy.topic_to_topology("myrealm/dt/myrealm-acme-area42-eui70b3d57ed005dac6/data.json")
assert topology == SmartMunch(
{
"realm": "myrealm",
"network": "acme",
"gateway": "area42",
"node": "eui70b3d57ed005dac6",
"slot": "data.json",
}
)
Copy link
Member Author

@amotl amotl May 25, 2023

Choose a reason for hiding this comment

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

@thiasB: When looking at 1, your devices are probably configured with device identifiers like device_id=hiveeyes-thias-freiland-hive2, so they do include the realm redundantly at the first device identifier segment. This test case demonstrates that it will be ignored, so that you do not have to re-program the devices.

Did I get this detail right?

Footnotes

  1. https://community.hiveeyes.org/t/tts-ttn-daten-an-kotori-weiterleiten/1422/35

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you are right. However, re-creating the devices in the TTN console wouldn't take much effort. If the solution below is active, I will be happy to not touch device registration. For new devices, I will follow the rule that the realm is not included in the device ID.

kotori/daq/strategy/wan.py Outdated Show resolved Hide resolved
kotori/daq/strategy/wan.py Outdated Show resolved Hide resolved
Add more details about per-device decoding to the `WanBusStrategy`, and
exercise a few topic/channel decodings, including edge cases.

Examples:

- myrealm/acme/area-42/foo-70b3d57ed005dac6
- myrealm/d/123e4567-e89b-12d3-a456-426614174000
- myrealm/dt/acme-area42-eui70b3d57ed005dac6
- myrealm/dt/eui-70b3d57ed005dac6
- myrealm/dt/acme-area42-eui70b3d57ed005dac6
- myrealm/dt/acme-area42-eui70b3d57ed005dac6-suffix
- myrealm/dt/myrealm-acme-area42-eui70b3d57ed005dac6
- mqttkit-1/d
- myrealm/dt
amotl added 2 commits June 7, 2023 00:15
For the new SensorWAN "direct" addressing scheme, by getting rid of the
previous `/d` vs. `/dt` path fragments, and using `/device` vs.
`/channel` instead, it reduces confusion about the meaning of those
prefixes, and makes usage less error prone.
By defining a `direct_channel_allowed_networks` setting on the
application configuration, the direct access to the corresponding
channel will be restricted to the specified networks/owners.
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