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

Support python3; refactor asynchat to use aiofile. #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yetab
Copy link

@yetab yetab commented Nov 27, 2024

Hi there!

Awesome work! I found python3 not working, so I did some patch work here.

I have updated the library to use python3 and refactored to use aiofile to resolve some compatibility issues.
I ran the unittests and they all passed.

If there is any issue, please let me know. Thanks.

@johnjohndoe
Copy link
Contributor

It would be great if pidcat would also be fixed for Python3.

@marshall
Copy link
Owner

@yetab thanks so much for the PR!

I haven't thought about this codebase in quite some time.. and was pleasantly surprised people are still interested in contributing :)

the code looks sane, and I'd be happy to merge, but I tried running the tests locally in a python3 venv, and saw these two errors. any chance you can take a look?

❯ ADB=/Users/marshall/Library/Android/sdk/platform-tools/adb ./test.py
............/opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/subprocess.py:1137: ResourceWarning: subprocess 93469 is still running
  _warn("subprocess %s is still running" % self.pid,
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/subprocess.py:829: ResourceWarning: unclosed file <_io.BufferedReader name=6>
  _cleanup()
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/subprocess.py:1137: ResourceWarning: subprocess 93470 is still running
  _warn("subprocess %s is still running" % self.pid,
ResourceWarning: Enable tracemalloc to get the object allocation traceback
E...F......
======================================================================
ERROR: test_stay_connected (logcat_color_test.LogcatColorAsyncTest.test_stay_connected)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/asyncio/base_events.py", line 720, in run_until_complete
    return future.result()
           ~~~~~~~~~~~~~^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/test/logcat_color_test.py", line 190, in test_stay_connected
    await self.lc.loop()
  File "/Users/marshall/Code/github.com/marshall/logcat-color/logcat-color", line 284, in loop
    await self.start()
  File "/Users/marshall/Code/github.com/marshall/logcat-color/logcat-color", line 280, in start
    await self.init_reader()
  File "/Users/marshall/Code/github.com/marshall/logcat-color/logcat-color", line 261, in init_reader
    line = await afp.readline()
           ^^^^^^^^^^^^^^^^^^^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/aiofile/utils.py", line 247, in readline
    chunk = await self.__read(self._READLINE_CHUNK_SIZE)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/aiofile/utils.py", line 227, in __read
    data = await self.file.read_bytes(length, self._offset)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/aiofile/aio.py", line 233, in read_bytes
    return await self.__context.read(size, self.fileno(), offset)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/caio/asyncio_base.py", line 58, in submit
    return op.get_value()
           ~~~~~~~~~~~~^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/caio/python_aio.py", line 241, in get_value
    raise self.exception
  File "/opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
                    ~~~~^^^^^^^^^^^^^^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/caio/python_aio.py", line 98, in _handle_read
    self.__pread(
    ~~~~~~~~~~~~^
        operation.fileno,
        ^^^^^^^^^^^^^^^^^
        operation.nbytes,
        ^^^^^^^^^^^^^^^^^
        operation.offset,
        ^^^^^^^^^^^^^^^^^
    ),
    ^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/.venv/lib/python3.13/site-packages/caio/python_aio.py", line 79, in __pread
    return os.pread(fd, size, offset)
           ~~~~~~~~^^^^^^^^^^^^^^^^^^
OSError: [Errno 29] Illegal seek

======================================================================
FAIL: test_piped_input (logcat_color_test.LogcatColorTest.test_piped_input)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/marshall/Code/github.com/marshall/logcat-color/test/logcat_color_test.py", line 19, in wrapped
    fn(self)
    ~~^^^^^^
  File "/Users/marshall/Code/github.com/marshall/logcat-color/test/logcat_color_test.py", line 84, in test_piped_input
    self.assertEqual(self.proc.returncode, 0)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 1 != 0

----------------------------------------------------------------------
Ran 23 tests in 0.689s

FAILED (failures=1, errors=1)

@zcattacz
Copy link

zcattacz commented Feb 3, 2025

Using the above fork, I am missing a lot of messages, packages filter seldom work since ActiveityManager's Proc start message seldom got received. Below adjustment got it fixed:
in logcat-color L267:

        async with async_open(self.input, 'r', context=linux_ctx) as afp:
            async for line in afp:
                self.reader.process_line(line)

also maybe Profile.include() should check for tag==ActivityManager before process_new_pid() to save some function calls.

Anyway thank you all for making a working filterable logcat for python3. It makes the development outside of Android Studio easier.

@zcattacz
Copy link

zcattacz commented Feb 3, 2025

The test.py showed all pass

...
----------------------------------------------------------------------
Ran 23 tests in 1.911s

OK

There is another issue, the adb path in config is not picked up correctly,
logcat-color L174

    def get_adb_prog(self):
        adb = "adb" # Let the system find adb on the PATH
        if "ADB" in os.environ:
            adb = os.environ["ADB"]
            return adb
        # `get_adb_devices()` also calls this but doesn't process config.
        # moved from `get_adb_args()`
        if config_adb:
            adb = config_adb
        return abd

@zcattacz
Copy link

zcattacz commented Feb 3, 2025

This is not py3 related, there is another thing that's acting weird, the filters seem to get first fail win.

    def include(...):
        ...
        for filter in self.filters:
            if not filter(data):
                return False

        return True

@zcattacz
Copy link

zcattacz commented Feb 3, 2025

This is not py3 related, the format is used as layout, and passed to logcat on the commandline, it seem impossible to filter by package/pid and use tag layout at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants