fbradyirl / openwrt-luci-rpc

Other
34 stars 22 forks source link

Added support for earlier OpenWRT versions #27

Closed edofullin closed 5 years ago

edofullin commented 5 years ago

I wanted to add support for my ChaosCalmer-basted router. To do that I passed the token in the url instead of using session cookies, this way should be compatible with both new and old openwrt versions.

I have also rewritten the _determine_if_legacy_version method, now it checks the version by checking the /etc/openwrt_version file and stores the version in a class attribute.

Things that need to be checked:

I hope people with older routers will be able to use this in home assistant. Sorry for my english, it is not my native language

Fixes #25

fbradyirl commented 5 years ago

Nice job and thank you for submitting this!!

I will try this out in a few days when I get time and will review then also. Thanks!

fbradyirl commented 5 years ago

I think if you change your exec command to run the following you should get the version successfully:

awk -F= '$1==\"VERSION\" { print $2 ;}' /etc/os-release

(also you will need to strip quotes from the result. e.g.

self.owrt_version = version.parse(content.replace("\"", "").strip())

Tested and this works fine with 18.06.

fbradyirl commented 5 years ago

I think if you change your exec command to run the following you should get the version successfully:

awk -F= '$1==\"VERSION\" { print $2 ;}' /etc/os-release

(also you will need to strip quotes from the result. e.g. self.owrt_version = version.parse(content.replace("\"", "").strip()) Tested and this works fine with 18.06.

Sorry, there is no file os-release in my version (probably because it is based on BusyBox).

Can you run cat /etc/openwrt_release? If yes post the output please, I'll see if I can get the version from there otherwise we rollback to your original method

Thanks!

Sure.

# cat /etc/openwrt_release
DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='18.06.4'
DISTRIB_REVISION='r7808-ef686b7292'
DISTRIB_TARGET='ramips/mt7621'
DISTRIB_ARCH='mipsel_24kc'
DISTRIB_DESCRIPTION='OpenWrt 18.06.4 r7808-ef686b7292'
DISTRIB_TAINTS=''
fbradyirl commented 5 years ago

"awk -F= '$1==\"DISTRIB_RELEASE\" { print $2 ;}' /etc/openwrt_release" seems to work for me.

fbradyirl commented 5 years ago

(you need to then strip the single quotes from that result.)

e.g. the try block contains:

            content = self._call_json_rpc(*rcp_sys_version_call,
                                          "awk -F= '$1==\"DISTRIB_RELEASE\" { print $2 ;}' /etc/openwrt_release")

            if content is None:
                raise LuciRpcUnknownError("could not \
                determine openwrt version")

            self.owrt_version = version.parse(content.replace("'", "").strip())
            log.info("Determined owrt_version {}".format(self.owrt_version))
fbradyirl commented 5 years ago

Also, can you authenticate your GitHub account with travis so the automatic build will kick off.

https://travis-ci.org/fbradyirl/openwrt-luci-rpc/pull_requests

edofullin commented 5 years ago

Unluckily /etc/openwrt_release in my router contains the version name instead of the version_id

DISTRIB_RELEASE='Chaos Calmer'
DISTRIB_REVISION='r46610'
DISTRIB_CODENAME='chaos_calmer'
DISTRIB_TARGET='brcm63xx-tch/VBNTK'
DISTRIB_DESCRIPTION='OpenWrt Chaos Calmer 15.05.1'
DISTRIB_TAINTS='no-all busybox'

However I started a 15.05 qemu vm and a 18.06 vm and I came to the following conclusions:

so the script is this: if [ -f \"/etc/os-release\" ]; then awk -F= '$1==\"VERSION_ID\" { print $2 ;}' /etc/os-release; else awk -F= '$1==\"DISTRIB_RELEASE\" { print $2 ;}' /etc/openwrt_release; fi | tr -d \\'\\\" (already json escaped)

this command checks if os-release exists and gets the version from there if it does otherwise it gets the version from openwrt_release using your command. I could just take the version from openwrt_release but os-release is actually the standard way to get infor about the distro in linux so I guess it's better to get the version from there if possible. It also pipes the result to tr which removes the quotes (either ' and " because both are used in different versions)

It seems to work with my vm and my router, I am going to fix tox and commit in a few minutes

edofullin commented 5 years ago

Also, can you authenticate your GitHub account with travis so the automatic build will kick off.

https://travis-ci.org/fbradyirl/openwrt-luci-rpc/pull_requests

I already have a travis account linked to my github account, do I need to do something? Sorry, it's the second time I create a pull request

fbradyirl commented 5 years ago

New changes look good on 18.06.

All good except for the new test file which is failing Tox

testDiscover (tests.test_openwrt_15.TestOpenwrt15LuciRPC) ... FAIL

======================================================================
FAIL: testDiscover (tests.test_openwrt_15.TestOpenwrt15LuciRPC)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/fibrady/projects/openwrt_luci_rpc/tests/test_openwrt_15.py", line 15, in testDiscover
    assert "HOST" in os.environ
AssertionError

----------------------------------------------------------------------
Ran 7 tests in 0.002s

FAILED (failures=1)

Were you planning on setting the environment variables somewhere in the pipeline?

fbradyirl commented 5 years ago

Also, can you authenticate your GitHub account with travis so the automatic build will kick off. https://travis-ci.org/fbradyirl/openwrt-luci-rpc/pull_requests

I already have a travis account linked to my github account, do I need to do something? Sorry, it's the second time I create a pull request

https://travis-ci.org/fbradyirl/openwrt-luci-rpc/requests is showing

Abuse detected

for your commits, so it is not triggering a build for this PR. From Googling, I see some people mentioning for the PR author to re-authenticate with travis perhaps can fix it.

Screenshot 2019-07-17 at 15 12 09
fbradyirl commented 5 years ago

See https://travis-ci.community/t/some-pr-are-not-analyzed-with-message-abuse-detected/1191

fbradyirl commented 5 years ago

I recommend deleting the tests.test_openwrt_15.TestOpenwrt15LuciRPC file as it adds no value unless you intend running a router somehow in the travis pipeline.

Then we can merge this and create a new build.

edofullin commented 5 years ago

New changes look good on 18.06.

All good except for the new test file which is failing Tox

testDiscover (tests.test_openwrt_15.TestOpenwrt15LuciRPC) ... FAIL

======================================================================
FAIL: testDiscover (tests.test_openwrt_15.TestOpenwrt15LuciRPC)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/fibrady/projects/openwrt_luci_rpc/tests/test_openwrt_15.py", line 15, in testDiscover
    assert "HOST" in os.environ
AssertionError

----------------------------------------------------------------------
Ran 7 tests in 0.002s

FAILED (failures=1)

Were you planning on setting the environment variables somewhere in the pipeline?

That was just for me, I set my router credentials as env variables to not push them on github, I'll delete the test

I recommend deleting the tests.test_openwrt_15.TestOpenwrt15LuciRPC file as it adds no value unless you intend running a router somehow in the travis pipeline.

Then we can merge this and create a new build.

Best choice would be to someway run a openwrt container / virtual machine on travis, for now I'll just comment out the test

Also, can you authenticate your GitHub account with travis so the automatic build will kick off. https://travis-ci.org/fbradyirl/openwrt-luci-rpc/pull_requests

I already have a travis account linked to my github account, do I need to do something? Sorry, it's the second time I create a pull request

https://travis-ci.org/fbradyirl/openwrt-luci-rpc/requests is showing

Abuse detected

for your commits, so it is not triggering a build for this PR. From Googling, I see some people mentioning for the PR author to re-authenticate with travis perhaps can fix it.

Screenshot 2019-07-17 at 15 12 09

I noticed today that my travis account (in which I don't log in for quite a while now) has been flagged for potential abuse (whatever that means). I have contacted their support and I'll keep you posted.

edofullin commented 5 years ago

Also, can you authenticate your GitHub account with travis so the automatic build will kick off. https://travis-ci.org/fbradyirl/openwrt-luci-rpc/pull_requests

I already have a travis account linked to my github account, do I need to do something? Sorry, it's the second time I create a pull request

https://travis-ci.org/fbradyirl/openwrt-luci-rpc/requests is showing

Abuse detected

for your commits, so it is not triggering a build for this PR. From Googling, I see some people mentioning for the PR author to re-authenticate with travis perhaps can fix it.

Screenshot 2019-07-17 at 15 12 09

Now cleared up, travis seems to pass correctly

fbradyirl commented 5 years ago

Good job!