-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fetch CPF configuration from GCS #9
Conversation
The default templates do not have class B enabled, for example, but IMO this should be good enough for "average user". Customization and more advanced configuration should be done manually anyway. |
86696ce
to
f2ef8e8
Compare
README.md
Outdated
4) RU864-RU_8CH | ||
5) RU864-RU_8CH_Legacy | ||
6) US915-US_8CH | ||
Please select a frequency plan template to use for lorad configuration.[0-6]2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do these come from? Kerlink?
I think we have to pull this from https://github.com/TheThingsNetwork/lorawan-frequency-plans, at least the index. Then we can use GCS (https://github.com/TheThingsNetwork/lorawan-stack/blob/master/pkg/gatewayconfigurationserver/gatewayconfigurationserver.go#L70) to get the JSON file. This does require an API key with gateway info rights.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just what is present in the vanilla CPF DOTA - the script then simply symlinks the template to current config. The user is expected to manually adapt the config at a later stage if necessary.
We could provide functionality for more advanced configuration(e.g. enabling and configuring class B) and/or get configuration from GCS.
I am not familiar with GCS, but looking at the source code the obvious thing is that the protocol used there has nothing to do with either lorad
, nor lorafwd
config required here. (Referring to https://github.com/TheThingsNetwork/lorawan-stack/blob/57a326a0c4f2bc81b896ede300baff2df199baf6/pkg/pfconfig/semtechudp/semtechudp.go#L34-L40)
What is the scope of GCS? It seems that this component is used to configure gateways using data in the stack. Do users configure this in the console? Does it include things like class B parameters etc.? If it is what I think it is, then we could potentially remove all the string replacements etc. and provision.sh
could just fetch both lorad
and lorafwd
configs from GCS given a (GCS) address and a key.
All of that is quite out of scope of this PR. Can you file an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of that is quite out of scope of this PR
What is in scope is ensuring that the right frequency plan is used. I don't care about vanilla CPF DOTA frequency plans; we have our own.
What is the format of those? Are those basically global_conf.json
?
As for the lorad
and lorafwd
config; they may very well override what is in global_conf.json
, assuming that CPF uses that format.
|
ae5ea30
to
e9e3fc1
Compare
provision.sh
Outdated
setLorafwdKeyQuoted "${sshCmd}" "node" "${stackAddr}" | ||
pushConfig "${gatewayAddr}" \ | ||
"${apiKey}" \ | ||
"${stackAddr}/api/v3/gcs/gateways/${gatewayID}/cpf/lorad/lorad.json" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cpf
is too generic ("common packet forwarder"); let's make sure we specifically indicate it's kerlink
here, like kerlink-cpf
Also, why are these specific folders?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was TheThingsNetwork/lorawan-stack#1747, but I guess we can still change this since we did not make a release.
The folders are different, because these are 2 independent services - lorad
and lorafwd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK fine. Yeah please change there
provision.sh
Outdated
printf "Setting LNS downlink port to ${downlinkPort}...\n" >&2 | ||
setLorafwdKey "${sshCmd}" "service.downlink" "${downlinkPort}" | ||
function printUsage { | ||
printf "Usage: $0 GATEWAY-ADDRESS STACK-ADDRESS GATEWAY-ID API-KEY\n" >&2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit; <gateway-address> <stack-address> ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very much inconsistent across different tools:
But in general, from what I have seen, usually:
[]
enclose optional parameters- no
[]
means that parameters are mandatory <>
is only used by Go
I used the suggested format, because most "old-school" Unix tooling uses that.
I don't really have a stong opinion about this, however I think <>
may be confusing, since that syntax is very uncommon(i.e. the only tool I know of using that syntax is Go compiler)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah OK I partly agree. In the ssh
and grep
cases, the command is mandatory and in [ ]
, though. If you execute them without a command, they just print help.
So maybe [gateway_address] [stack_address] ...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only mandatory part of ssh
is destination
, which is without []
In case of grep
, the only mandatory part is the pattern
, which is, again, without []
Note that FILE
is not mandatory for grep
:
$ echo "asd" | grep
Usage: grep [OPTION]... PATTERNS [FILE]...
Try 'grep --help' for more information.
$ echo "asd" | grep "a"
asd
See SSH as well:
rvolosatovs@cobalt ~ % ssh oxygen
Last login: Thu Dec 19 12:43:52 2019 from <redacted>
rvolosatovs@oxygen ~ %
@rvolosatovs looks like this can be updated, reviewed and merged |
d06c946
to
a87a11a
Compare
Refs TheThingsNetwork/lorawan-stack#1716 (comment)
Configure frequency plan after CPF installation in provisioning script192.168.4.155
, which is the gateway's default IPExample(with some error handling as well):EDIT: removed original example, since it's not applicable anymore