Add low-level command for direct raw memory access#43
Add low-level command for direct raw memory access#43johannessen wants to merge 1 commit intocr:devfrom
Conversation
| help = "Read/write hex bytes in the device memory (advanced feature, use caution)" | ||
|
|
||
| @staticmethod | ||
| def setup_args(parser) -> None: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| help = "Read/write hex bytes in the device memory (advanced feature, use caution)" | ||
|
|
||
| @staticmethod | ||
| def setup_args(parser) -> None: |
There was a problem hiding this comment.
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.
| from . import id | ||
| from . import info | ||
| from . import nmea | ||
| from . import poke |
There was a problem hiding this comment.
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?
| 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)") |
There was a problem hiding this comment.
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?
This proposed addition allows writing and reading raw hexadecimal data to/from individual memory addresses in the device from the command line.
It’s something I found quite useful when working on the GX1400 last year. With this tool it’s really easy to make small changes on the device, in a lot less time than the usual dump–editor–flash cycle would require.
While this powerful tool also makes it easy to corrupt the device’s memory when used carelessly, the readme’s disclaimer already has a clear warning about the associated risk. I think this tool is valuable enough that the risk is outweighed by the benefits.