ser2net: Added support for config file. #5302
Signed-off-by: Jasper Scholte <[email protected]>
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| config 'proxy' | ||
| option 'enabled' '0' | ||
| option 'port' '8000' | ||
| option 'protocol' 'raw' | ||
| option 'timeout' '30' | ||
| option 'device' '/dev/ttyUSB0' | ||
| option 'options' '9600 1STOPBIT 8DATABITS' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| #!/bin/sh /etc/rc.common | ||
| # Copyright (C) 2017 OpenWrt.org | ||
|
|
||
| START=75 | ||
| STOP=10 | ||
| CONFIGFILE="/tmp/ser2net.conf" | ||
|
|
||
| USE_PROCD=1 | ||
| PROG=/usr/sbin/ser2net | ||
|
|
||
| start_service() { | ||
| config_load 'ser2net' | ||
|
|
||
| local enabled | ||
| config_get_bool enabled config 'enabled' '0' | ||
| [ "$enabled" -gt 0 ] || return 1 | ||
|
There was a problem hiding this comment. You want the enable to be on the use of the UCI conversion. There's still a gain to be had from an functional init script with the existing /etc/ser2net.conf file. particularly when you'll never be able to match the depth of availlable config in the UCI model. Like https://github.com/openwrt/packages/blob/master/net/mosquitto/files/etc/init.d/mosquitto#L186-L191 There was a problem hiding this comment. So what you meant is: Is my assumption correct? There was a problem hiding this comment. yes, that's what I was talking about at least. There was a problem hiding this comment. Seems feasible to me. Do I have to provide a default /etc/ser2net.conf file or can I just check for its existence and fail if not found? There was a problem hiding this comment. well, the package is already installing one, so I wouldn't bother doing any further checking. There was a problem hiding this comment. Unless the user explicitly removes it... There was a problem hiding this comment. @pprindeville What if they remove the package itself?! where do you stop? There was a problem hiding this comment. Let's not go over the top. For someone concerned about security and not having a "fail open" policy, removing defaults to stop a (unconfigured) service from running is good security practice. |
||
|
|
||
| ser2net_create_config config || return 1 | ||
| procd_open_instance | ||
| procd_set_param command "$PROG" -n -c "$CONFIGFILE" | ||
| procd_close_instance | ||
| } | ||
|
|
||
| ser2net_create_config() { | ||
| local cfg=$1 | ||
| local port | ||
| local device | ||
|
|
||
| config_get port $cfg port | ||
| config_get device $cfg device | ||
| [ -z "$port" -o -t "$device" ] && return 1 | ||
|
|
||
| local protocol | ||
| local timeout | ||
| local options | ||
|
|
||
| config_get protocol $cfg protocol | ||
|
There was a problem hiding this comment. this is called "state" in the ser2net configs, is there any real good reason to use a different word here? There was a problem hiding this comment. This was requested by @mhei : #5302 (comment) |
||
| config_get timeout $cfg timeout | ||
| config_get options $cfg options | ||
|
|
||
| if [ -z "$options" ]; then | ||
| echo "$port":"$protocol":"$timeout":"$device" >> "$CONFIGFILE | ||
| else | ||
| echo "$port":"$protocol":"$timeout":"$device":"$options" >> "$CONFIGFILE" | ||
| fi | ||
| } | ||
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.
Hm, "proxy" is not my first choice, but in the absence of a better idea... The important thing is, that we might add additional types later, e.g. for LED configuration etc.
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.
Why not just device? one section for each device line?
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.
there is already a parameter device? I think changing it to device makes it confusing. And I suppose ser2net is proxying the serial data to network data. So it seems ok to me?
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 port # and the device name are unique strings... I would differentiate the blocks based on either one of those, and make the other one a mandatory parameter.
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.
well, they're not actually. ser2net can have the same port and multiple devices, or multiple ports for the same device, and have them enabled/disabled separately at runtime via the control port.