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

Derive InfluxDB database and measurement from dev_id and signal data from all gateways #2

Merged
merged 20 commits into from Jan 28, 2020

Conversation

thiasB
Copy link
Contributor

@thiasB thiasB commented Nov 25, 2019

  • fix for ERROR: 'collections.OrderedDict' object has no attribute 'iteritems'
  • optional: dynamic InfluxDB database and measurement derived from dev_id, e.g. when more than one device registered in TTN App
  • store LoRa signal quality data from all receiving gateways

@thiasB thiasB changed the title Dictionaries do not have iteritem methods in 3.x. Derive InfluxDB database and measurement from dev_id Nov 25, 2019
@thiasB thiasB changed the title Derive InfluxDB database and measurement from dev_id Derive InfluxDB database and measurement from dev_id and signal data from all gateways Nov 28, 2019
@amotl
Copy link
Member

amotl commented Nov 29, 2019

Thanks for your contributions, @thiasB. Much appreciated!

When looking at the changes, two things come to my mind when operating this piece in a generic / arbitrary TTN environment.

  1. What happens when the InfluxDB database and measurement name can't be derived from the dev_id like currently implemented within this PR? Maybe we should be more graceful here and use the implemented split-scheme just when activated through a specific command line option.

  2. When enumerating all gateways seen by the device, we might want to turn the telemetry fields within the downstream message payload into some stable scheme, e.g. by keying them with the gtw_id. I propose using these telemetry field names:

    • gw_{gtw_id}_rssi
    • gw_{gtw_id}_snr
    • gw_{gtw_id}_latitude
    • gw_{gtw_id}_longitude
    • gw_{gtw_id}_altitude

    The gtw_id should probably be interpolated in lowercase letters.

What do you think about this?

@thiasB
Copy link
Contributor Author

thiasB commented Dec 3, 2019

yes, if you want to keep the original generic behavior then make the "hiveeyes scheme" optional. feel free to implement :)

Geographical data is really not needed here. Usually they don't change over time. Let's not artificially blow up databases. Instead spreading factor (SF) and bandwidth (BW) could be candidates for consideration.


# Convert all numeric values to floats.
data = convert_floats(data)
data = convert_floats(data, integers="gtw_count,sf,bw")
Copy link
Member

Choose a reason for hiding this comment

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

You should pass a list here, like integers=["gtw_count", "sf", "bw"].

@thiasB
Copy link
Contributor Author

thiasB commented Jan 16, 2020

Will do, though it's working.

@amotl
Copy link
Member

amotl commented Jan 28, 2020

Thanks for addressing the generic aspects of ttnlogger by introducing respective command line arguments to control these details.

@thiasB
Copy link
Contributor Author

thiasB commented Jan 28, 2020

You're welcome.
Squash into single commit?

Comment on lines 124 to 133
for i in range(num_gtws):
gtw_id = ttn_message.metadata.gateways[i].gtw_id
print('Gateway ' + str(i+1) + ' ID : ' + gtw_id)
key_rssi = 'gw_' + gtw_id + '_rssi'
key_snr = 'gw_' + gtw_id + '_snr'
data[key_rssi] = int(ttn_message.metadata.gateways[i].rssi)
data[key_snr] = float(ttn_message.metadata.gateways[i].snr)

data['sf'] = int(ttn_message.metadata.data_rate.split('BW')[0][2:])
data['bw'] = int(ttn_message.metadata.data_rate.split('BW')[1])
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite see through that yet.

Copy link
Contributor Author

@thiasB thiasB Jan 28, 2020

Choose a reason for hiding this comment

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

Messages can be received from several gateways at once and each gateway puts the individual signal quality data (RSSI and SNR) into the final message. Spreading factor (sf) and bandwidth (bw) are determinated by the sending device and thus equals for every receiver.

Copy link
Contributor Author

@thiasB thiasB Jan 28, 2020

Choose a reason for hiding this comment

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

And, on multiple receiving gateways there's no defined order of appearance in the TTN message. Thus, always picking gw0 will point to random gateways. That's why I'm now looping through and forwarding data from all gateways

Copy link
Member

Choose a reason for hiding this comment

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

On multiple receiving gateways there's no defined order of appearance in the TTN message.

That's sad. But we are keying by gtw_id now, right? This contains stable identifiers like eui-3338333018004700.

Copy link
Member

Choose a reason for hiding this comment

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

Spreading factor (sf) and bandwidth (bw) are determinated by the sending device and thus equals for every receiver.

So, the last two lines actually decode the

"data_rate": "SF7BW125"

right?

@amotl
Copy link
Member

amotl commented Jan 28, 2020

Geographical data is really not needed here. Usually they don't change over time. Let's not artificially blow up databases.

I believe it won't do any harm. Unfortunately, if we don't have it within the database, we can't visualize it.

@thiasB
Copy link
Contributor Author

thiasB commented Jan 28, 2020

55aae7b is yet untested

@thiasB thiasB requested a review from amotl January 28, 2020 13:42
if not self.nogeo:
key_lon = 'gw_' + gtw_id + '_lon'
key_alt = 'gw_' + gtw_id + '_alt'
data[key_lat] = float(ttn_message.metadata.gateways[i].latitude)
Copy link
Member

Choose a reason for hiding this comment

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

key_lat might be undefined here.

@amotl
Copy link
Member

amotl commented Jan 28, 2020

Squash into single commit?

Do you mean I should squash it when merging this pull request?

@amotl amotl merged commit cd828ef into mqtt-tools:master Jan 28, 2020
@amotl
Copy link
Member

amotl commented Jan 28, 2020

Thanks!

@thiasB
Copy link
Contributor Author

thiasB commented Jan 28, 2020

wasn't done testing and will push some more fixes :)

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

2 participants