search

christianrowlands / android-network-survey

Cellular Survey Android App
https://www.networksurvey.app
Apache License 2.0
131 stars 25 forks source link

Inconsistent naming for mqtt #18

Closed tranzmatt closed 1 year ago

tranzmatt commented 1 year ago

Can we get consistent with the naming convention on the MQTT topics? Some you have as 80211_ (eg, 80211_beaconmessage), other as wifi (wifi_deauthentication_message), but all of them use a Wifi*Record (eg, WifiDeauthenticationRecord), so this makes things inconsistent and pain of building a message to send. IDC which, just one of the other, please.

christianrowlands commented 1 year ago

@tranzmatt , great point! Normally I catch things like that but missed this one, so thanks for calling it out.

I would like to standardize on the wifi_* prefix since it is shorter and does not contain numbers. And as you point out, the record names on those topics all use WiFi. The problem with making the change now is that the 80211_beacon_message topic has been out in the wild for quite some time, and the NS app has used that as the MQTT topic for a while. I know several deployed backends that would require some updates to support the topic rename.

I don't know many systems that are using wifi_ota_message and wifi_deauthentication_message was just added a few days ago so I think changing those to have the 80211_* prefix might be the path of least resistance. Does that work for you @tranzmatt ?

@glencornell , are you ok if I make that change (change to use the 80211_ prefix on the two messages you added)?

glencornell commented 1 year ago

Yes. I’m good with changing the names as you described above. Apologies to all: git-blame me for introducing the inconsistent names.

christianrowlands commented 1 year ago

Here is the PR for this change: https://github.com/christianrowlands/network-survey-messaging/pull/16

christianrowlands commented 1 year ago

Version 0.10.0 of the NS Messaging API has been released. I went with the approach of using the 80211_* prefix for the topic names, and wifi_* prefix for the records themselves. I mostly went with this because the original beacon message is done this way and it would be really hard to make that breaking change now. Also, worthy of note is that you can't start the record names with a number (e.g. 8) because the generated Java classes would not compile (java does not allow you to start classes with a number). I agree the idea scenario would be to start everything with wifi_*, but I think what we have now is good enough.

@tranzmatt , let me know if you have any thoughts, ideas, or questions.