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

[change] ModemManager device option needs to update to ctl_device #70

Open
mips171 opened this issue Sep 19, 2021 · 6 comments
Open

[change] ModemManager device option needs to update to ctl_device #70

mips171 opened this issue Sep 19, 2021 · 6 comments
Labels
enhancement New feature or request hacktoberfest Easy issues for attracting Hacktoberfest participants.

Comments

@mips171
Copy link

mips171 commented Sep 19, 2021

Hey,

Just letting you know, we are changing this from device to ctl_device upstream in the OpenWrt ModemManager protocol handler because of compatibility issues with DSA. I will link the PR here when it is in.

local modem = uci_cursor.get('network', interface['interface'], 'device')

@mips171
Copy link
Author

mips171 commented Sep 29, 2021

How's this look? It looks like the uci_cursor.get returns nil if that entry doesn't exist, right?

        local compat_device = uci_cursor.get('network', interface['interface'], 'device')
        local device = uci_cursor.get('network', interface['interface'], 'ctl_device')
        local modem = (compat_device or device)

        if (compat_device and device) then
            modem = device
        end

        local info = {}

        local general = io.popen('mmcli --output-json -m '..modem):read("*a")

@nemesifier
Copy link
Member

@nickberry17 yes, it should, I haven't tested the following modified version yet but if it works is a bit shorter but still readable:

local device = uci_cursor.get('network', interface['interface'], 'ctl_device') or \
               uci_cursor.get('network', interface['interface'], 'device')
local info = {}
local general = io.popen('mmcli --output-json -m '..device):read("*a")

The second lineand the or won't be executed in the future and can be removed at some point in the future.

@nemesifier nemesifier changed the title ModemManager device option needs to update to ctl_device [change] ModemManager device option needs to update to ctl_device Sep 29, 2021
@nemesifier nemesifier added the enhancement New feature or request label Sep 29, 2021
@nemesifier nemesifier added this to Backlog in OpenWISP Priorities for next releases via automation Sep 29, 2021
@nemesifier nemesifier added this to To do (general) in OpenWISP Contributor's Board via automation Sep 29, 2021
@nemesifier nemesifier added the hacktoberfest Easy issues for attracting Hacktoberfest participants. label Sep 29, 2021
@mips171
Copy link
Author

mips171 commented Sep 29, 2021

Is it an inclusive or? Do we need to check if both are set, so we don't get device set to /sys/devices/platform/soc/8af8800.usb3/8a00000.dwc3/xhci-hcd.0.auto/usb2/2-1/sys/devices/platform/soc/8af8800.usb3/8a00000.dwc3/xhci-hcd.0.auto/usb2/2-1 In a sysupgrade scenario where settings are kept, both device and ctl_device could be set.

@nemesifier
Copy link
Member

Is it an inclusive or? Do we need to check if both are set, so we don't get device set to /sys/devices/platform/soc/8af8800.usb3/8a00000.dwc3/xhci-hcd.0.auto/usb2/2-1/sys/devices/platform/soc/8af8800.usb3/8a00000.dwc3/xhci-hcd.0.auto/usb2/2-1 In a sysupgrade scenario where settings are kept, both device and ctl_device could be set.

The one with precedence should come first, the or is executed only if the first value is nil.

@mips171
Copy link
Author

mips171 commented Sep 29, 2021

@nickberry17 yes, it should, I haven't tested the following modified version yet but if it works is a bit shorter but still readable:

local device = uci_cursor.get('network', interface['interface'], 'ctl_device') or \
               uci_cursor.get('network', interface['interface'], 'device')
local info = {}
local general = io.popen('mmcli --output-json -m '..device):read("*a")

The second lineand the or won't be executed in the future and can be removed at some point in the future.

Didn't like the \

uci: Entry not found
lua: /usr/sbin/netjson-monitoring:326: unexpected symbol near '\'

Putting them on the same line:

uci: Entry not found
lua: /usr/sbin/netjson-monitoring:330: attempt to concatenate global 'modem' (a nil value)
stack traceback:
	/usr/sbin/netjson-monitoring:330: in function 'specialized_info'
	/usr/sbin/netjson-monitoring:392: in function 'get_interface_info'
	/usr/sbin/netjson-monitoring:578: in main chunk
	[C]: ?

The longhand way works though

@nemesifier nemesifier moved this from To do (general) to To do (OpenWRT) in OpenWISP Contributor's Board Oct 1, 2021
@devkapilbansal
Copy link
Member

Putting them on the same line:

uci: Entry not found
lua: /usr/sbin/netjson-monitoring:330: attempt to concatenate global 'modem' (a nil value)
stack traceback:
	/usr/sbin/netjson-monitoring:330: in function 'specialized_info'
	/usr/sbin/netjson-monitoring:392: in function 'get_interface_info'
	/usr/sbin/netjson-monitoring:578: in main chunk
	[C]: ?

What's the modem here? I think it is renamed to device in other prior instances expect one giving error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hacktoberfest Easy issues for attracting Hacktoberfest participants.
Development

No branches or pull requests

3 participants