Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion hxtool/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from . import id
from . import info
from . import nmea
from . import poke
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In another comment, you suggested the CLI to look like this:

hxtool poke ...
hxtool peek ...

I really like that a lot, I’ve been catching myself typing peek into the terminal window all the time, even though it doesn’t actually work. 😅 I just wasn’t bold enough to split this feature into two entirely separate commands, that’s all.

Is there a way to make peek an alias of poke? Or do we need two entirely separate command files peek.py and poke.py?


__all__ = [
"run",
Expand All @@ -17,5 +18,6 @@
"gpslog",
"id",
"info",
"nmea"
"nmea",
"poke",
]
103 changes: 103 additions & 0 deletions hxtool/cli/poke.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# -*- coding: utf-8 -*-

from argparse import ArgumentError, ArgumentTypeError
from binascii import hexlify, unhexlify
from logging import getLogger
import os
import re

import hxtool
from .base import CliCommand

logger = getLogger(__name__)


def default_length():
try:
length = os.environ.get("HXTOOL_PEEK_LENGTH", "1")
return hexadecimal_number(length)
except (ArgumentTypeError, ValueError):
return 1


def hexadecimal_number(string):
number = int(string, 16)
if number < 0:
raise ArgumentTypeError("must be positive")
return number


def hexadecimal_data(string):
return unhexlify(bytes(re.sub(r"\A0x", "", string), 'utf-8'))


class PokeCommand(CliCommand):

name = "poke"
help = "Read/write hex bytes in the device memory (advanced feature, use caution)"

@staticmethod
def setup_args(parser) -> None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not happy with the argument logic here. If you specify --data, then what is --length supposed to do? I suggest the following:

hxtool poke --start 0x100 --data 1234 with both args mandatory

and

hxtool peek --start 0x100 --end 0x120 or
hxtool peek --start 0x100 --lenght 0x20, so mandatory start and either end or length.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About --length:

For poke, --length is extremely useful as a sanity check. One could well argue that --length should even be mandatory for poke.

However, for peek, omitting --length is not a problem at all. You can just read a default number of bytes from the device. The current implementation defaults to a single byte for no particular reason, but allows users to change that default through an environment variable.


parser.add_argument('offset',
type=hexadecimal_number,
help="hex address to poke/peek")

parser.add_argument('data',
nargs='?',
type=hexadecimal_data,
help="data to poke (when omitted: peek, don't poke)")
Comment on lines +42 to +49
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two are currently positional arguments. In another comment, you suggested to change both to named arguments (--start and --data).

I don’t like that idea very much. I don’t think it’s necessary: The arguments to poke will always be a memory location and the data to write. And this is the natural order for these arguments. I think it’s just more typing, without any advantage.

You also suggested --end as an alternative for --length. Could you give an example of a memory location where such an option would be useful?

I’m concerned about the ambiguity: Is --end the address of the last byte to be written, or that of the byte after the last one? No big deal for peek, but an easy way to get a mess with poke. So, if there is an option like this, it should be named more clearly; maybe --last?


parser.add_argument("-l", "--length",
type=hexadecimal_number,
help="hex number of bytes to poke/peek")

def run(self):
hx = hxtool.get(self.args)
if hx is None:
return 10

if not hx.comm.cp_mode:
logger.critical("Handset not in CP mode (MENU + ON)")
return 11

offset = self.args.offset
data = self.args.data
length = self.args.length

if data is None:
# peek
if length is None:
length = default_length()
else:
# poke
if length is None:
length = len(data)
elif len(data) > length:
data = data[0:length]
logger.warning(f"Truncating data to "
f"{'0x%x' % length} byte{'s' if length != 1 else ''}")
elif len(data) < length:
raise ArgumentError(None, f"Data to poke is shorter than {'0x%x' % length} bytes")

if length > hx.config.CHUNK_SIZE:
raise ArgumentError(None,
f"Can't {'peek' if data is None else 'poke'} "
f"more than {'0x%X' % hx.config.CHUNK_SIZE} bytes at once "
f"on {type(hx).__name__}")
if offset + length > hx.config.CONFIG_SIZE:
raise ArgumentError(None,
f"Can't {'peek' if data is None else 'poke'} "
f"past the end of the {type(hx).__name__}'s memory "
f"at offset {'0x%X' % hx.config.CONFIG_SIZE}")

hx.comm.sync()

if data is None:
print(hexlify(hx.comm.read_config_memory(offset, length)).decode())
else:
logger.info(f"Writing {hexlify(data)} to device at offset {'0x%04x' % offset}")
hx.comm.write_config_memory(offset, data)

logger.info("Operation successful")
return 0
58 changes: 58 additions & 0 deletions tests/poke_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# -*- coding: utf-8 -*-

import argparse
import pytest

from hxtool.main import main


def test_hxtool_peek(capsys):
ret = main("--simulator -t 0 poke 0100 --length 0x6".split())
assert ret == 0

outerr = capsys.readouterr()
assert "Operation successful" in outerr.err
assert "414d3035374e\n" == outerr.out, "read HX870 flash id"


def test_hxtool_peek_no_length(capsys):
ret = main("--simulator -t 0 poke 1c".split())
assert ret == 0

outerr = capsys.readouterr()
assert "Operation successful" in outerr.err
assert "ff\n" == outerr.out, "peek length defaults to 1"


def test_hxtool_poke(capsys):
ret = main("--debug --simulator -t 0 poke 1234 aBCd".split())
assert ret == 0

outerr = capsys.readouterr()
assert "Operation successful" in outerr.err
assert """CP simulator processing message b'#CEPWR\\t1234\\t02\\tABCD\\t72\\r\\n'""" in outerr.err


def test_hxtool_poke_truncate(capsys):
ret = main("--debug --simulator -t 0 poke 10 0x01020304 -l 1".split())
assert ret == 0

outerr = capsys.readouterr()
assert "Truncating data to 0x1 byte" in outerr.err
assert "Operation successful" in outerr.err
assert """CP simulator processing message b'#CEPWR\\t0010\\t01\\t01\\t71\\r\\n'""" in outerr.err


def test_hxtool_peek_too_long_data():
with pytest.raises(argparse.ArgumentError):
main("--simulator -t 0 poke 005a -l 41".split())


def test_hxtool_poke_too_short_data():
with pytest.raises(argparse.ArgumentError):
main("--simulator -t 0 poke 12 00 -l 2".split())


def test_hxtool_poke_too_high_address():
with pytest.raises(argparse.ArgumentError):
main("--simulator -t 0 poke 7fff 0100".split())