-
Notifications
You must be signed in to change notification settings - Fork 13
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
HexEditor #10
base: master
Are you sure you want to change the base?
HexEditor #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I don't use java anymore so I can provide just some feedback and what I would like to see in my original project.
It would be nice to have not just parser but also writer functionality in the library. Not sure about the name of the library. :-D
But, I don't like much the way you mix the parsing and writing. IMHO, there should be separate functionality that has e.g. ability to convert byte[]
or MemoryRegions
and happen to be able to also receive streaming input from the parser if necessary. This should be somehow part of the library so the CLI tools are just using existing functionality.
@@ -35,6 +35,7 @@ public class Record { | |||
public int address; | |||
public RecordType type; | |||
public byte[] data; | |||
public byte checksum; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to have a method public byte getChecksum()
to calculate checksum here?
|
||
if (args.length == 0) { | ||
System.out.println("usage:"); | ||
System.out.println(" hexEditor <hex_in> <hex_out> -replace <start address> <data>"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I really don't understand what it should do and how to use it.
String hex = Integer.toHexString(inputArray[i] & 0xFF).toUpperCase().trim(); | ||
sb.append(new String(new char[2 - hex.length()]).replace('\0', '0')) | ||
.append(hex) | ||
.append(determiner); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usually, it is called delimiter
@@ -1,16 +1,16 @@ | |||
/** | |||
* Copyright (c) 2015, Jan Breuer All rights reserved. | |||
* | |||
* <p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please no html tags in license
public String getLine(Record record) { | ||
|
||
StringBuilder sb = new StringBuilder(":"); | ||
get(sb, intTo1BytesArr(record.length)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is somehow strange. get
is really not a good name for the method.
It would be better to add BufferedWriter
as a parameter and directly write to it instead of an intermediate StringBuilder.
String fileIn = "input.hex"; | ||
|
||
if (args.length == 0) { | ||
System.out.println("Just print binary data from IntelHex file to console"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid filler words like "just".
System.out.println("Just print binary data from IntelHex file to console"); | ||
System.out.println(); | ||
System.out.println("usage:"); | ||
System.out.println(" PrintBin <file_in>"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usual name for this functionality is hexdump.
StringBuilder sb = new StringBuilder(); | ||
|
||
for (int i = 0; i < inputArray.length; i++) { | ||
String hex = Integer.toHexString(inputArray[i] & 0xFF).toUpperCase().trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same problem as in HexWriter
|
||
for (int i = 0; i < inputArray.length; i++) { | ||
String hex = Integer.toHexString(inputArray[i] & 0xFF).toUpperCase().trim(); | ||
sb.append(new String(new char[2 - hex.length()]).replace('\0', '0')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xxd
style output make more sense form me in tools like this.
try (BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(destination))) { | ||
|
||
//cuts data to 16 bytes blocks | ||
//if 0xFF are redundant, skip it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is somehow hard to understand
I have a another thing. I do someting as a simple HEX editor. The editor is able replace or add some of bytes and save it back to the hex file. The merge is not ready yet, I must do more test with real data. Just look around and say if it is interenting for merge or not.
PS: I will do Bin2Hex too.
Thanks