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

Terminal write and erase improvements #1320

Merged
merged 23 commits into from
Apr 6, 2023
Merged
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6e9a978
Use ... in terminal write to specify end of memory
MCUdude Mar 17, 2023
6aa442b
Extend erase command
MCUdude Mar 17, 2023
f048046
Add support for negative address and length values
MCUdude Mar 18, 2023
f544331
Add support for negative address and length in cmd_dump.
MCUdude Mar 18, 2023
a0bc540
Fix incorrect length when using negative numbers
MCUdude Mar 19, 2023
8877942
tweak length byte + handle zero and negative length differently
MCUdude Mar 19, 2023
d783fa1
Handle zero and negative effective lengths
MCUdude Mar 19, 2023
745939f
Minor code improvements
MCUdude Mar 20, 2023
be2ba0d
Remove deprached ... ellipsis in cmd_dump
MCUdude Mar 20, 2023
e30e802
Update help text
MCUdude Mar 21, 2023
eefa904
Update docs
MCUdude Mar 22, 2023
b6d3429
Merge branch 'main' into terminal-write-end-mem
MCUdude Mar 22, 2023
fbee16b
Update terminal examples
MCUdude Mar 22, 2023
ae0d7c6
explain "pos" and "neg"
MCUdude Mar 22, 2023
0865e75
Update terminal r/w/erase examples
stefanrueger Mar 23, 2023
c74bbc2
Improve terminal read/write documentation
stefanrueger Mar 23, 2023
ff618b1
Improve terminal read usage notice
stefanrueger Mar 23, 2023
32f59d7
Clarify aliases for terminal commands
stefanrueger Apr 2, 2023
936d7ea
Review and remove surplus \n in the terminal and progress reporting
stefanrueger Apr 2, 2023
eb2e346
Fix return value of msg_xyz() functions
stefanrueger Apr 2, 2023
0a5d1eb
Remove double newline from terminal sig and send commands
stefanrueger Apr 3, 2023
5facea9
Update terminal examples
MCUdude Apr 4, 2023
9a7d532
Update command line invocation examples
MCUdude Apr 4, 2023
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
80 changes: 68 additions & 12 deletions src/term.c
MCUdude marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -287,17 +287,23 @@ static int cmd_dump(PROGRAMMER *pgm, AVRPART *p, int argc, char *argv[]) {
// Get start address if present
char *end_ptr;
if (argc >= 3 && strcmp(argv[2], "...") != 0) {
unsigned long ul = strtoul(argv[2], &end_ptr, 0);
int addr = strtol(argv[2], &end_ptr, 0);
if(*end_ptr || (end_ptr == argv[2])) {
pmsg_error("(dump) cannot parse address %s\n", argv[2]);
return -1;
}
if(ul > INT_MAX || ul >= (unsigned long) maxsize) {
pmsg_error("(dump) %s address 0x%lx is out of range [0, 0x%0*x]\n", mem->desc, ul,
mem->size > 0x10000? 5: 4, maxsize-1);

// Turn negative addr value (counting from top and down) into an actual memory address
if (addr < 0)
addr = maxsize + addr;

if (addr < 0 || addr >= maxsize) {
int digits = mem->size > 0x10000? 5: 4;
pmsg_error("(dump) %s address %s is out of range [-0x%0*x, 0x%0*x]\n",
mem->desc, argv[2], digits, maxsize, digits, maxsize-1);
MCUdude marked this conversation as resolved.
Show resolved Hide resolved
return -1;
}
read_mem[i].addr = (int) ul;
read_mem[i].addr = addr;
}

// Get no. bytes to read if present
Expand All @@ -307,14 +313,22 @@ static int cmd_dump(PROGRAMMER *pgm, AVRPART *p, int argc, char *argv[]) {
read_mem[i].addr = 0;
read_mem[i].len = maxsize - read_mem[i].addr;
} else if (argc == 4) {
unsigned long ul = strtoul(argv[3], &end_ptr, 0);
int len = strtol(argv[3], &end_ptr, 0);
if (*end_ptr || (end_ptr == argv[3])) {
pmsg_error("(dump) cannot parse length %s\n", argv[3]);
return -1;
}
if (ul == 0 || ul > INT_MAX) // Not positive if used as int, make it 1
ul = 1;
read_mem[i].len = (int) ul;
// Turn negative len value (no. bytes from top of memory) into an actual length number
if (len < 0)
len = maxsize + len + 1 - read_mem[i].addr;

if (len == 0)
return 0;
else if (len < 0) {
stefanrueger marked this conversation as resolved.
Show resolved Hide resolved
pmsg_error("(dump) invalid length %d\n", len);
stefanrueger marked this conversation as resolved.
Show resolved Hide resolved
return -1;
}
read_mem[i].len = len;
}
}
// Wrap around if the memory address is greater than the maximum size
Expand Down Expand Up @@ -457,14 +471,20 @@ static int cmd_write(PROGRAMMER *pgm, AVRPART *p, int argc, char *argv[]) {
int maxsize = mem->size;

char *end_ptr;
int addr = strtoul(argv[2], &end_ptr, 0);
int addr = strtol(argv[2], &end_ptr, 0);
if (*end_ptr || (end_ptr == argv[2])) {
pmsg_error("(write) cannot parse address %s\n", argv[2]);
return -1;
}

// Turn negative addr value (counting from top and down) into an actual memory address
if (addr < 0)
addr = maxsize + addr;

if (addr < 0 || addr >= maxsize) {
pmsg_error("(write) %s address 0x%05x is out of range [0, 0x%05x]\n", mem->desc, addr, maxsize-1);
int digits = maxsize > 0x10000? 5: 4;
pmsg_error("(write) %s address 0x%0*x is out of range [-0x%0*x, 0x%0*x]\n",
mem->desc, digits, addr, digits, maxsize, digits, maxsize-1);
return -1;
}

Expand All @@ -479,12 +499,22 @@ static int cmd_write(PROGRAMMER *pgm, AVRPART *p, int argc, char *argv[]) {
if (strcmp(argv[argc - 1], "...") == 0) {
write_mode = WRITE_MODE_FILL;
start_offset = 4;
len = strtoul(argv[3], &end_ptr, 0);
len = strtol(argv[3], &end_ptr, 0);
if (*end_ptr || (end_ptr == argv[3])) {
pmsg_error("(write ...) cannot parse length %s\n", argv[3]);
free(buf);
return -1;
}
// Turn negative len value (no. bytes from top of memory) into an actual length number
if (len < 0)
len = maxsize + len - addr + 1;
if (len == 0)
return 0;
if (len < 0 || len > maxsize - addr) {
pmsg_error("(write) effective %s start address 0x%0*x and effective length %d not compatible with memory size %d\n",
mem->desc, maxsize > 0x10000? 5: 4, addr, len, maxsize);
return -1;
stefanrueger marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
write_mode = WRITE_MODE_STANDARD;
start_offset = 3;
Expand Down Expand Up @@ -797,6 +827,32 @@ static int cmd_send(PROGRAMMER *pgm, AVRPART *p, int argc, char *argv[]) {


static int cmd_erase(PROGRAMMER *pgm, AVRPART *p, int argc, char *argv[]) {
if (argc > 4 || argc == 3) {
msg_error("Usage: erase <memory> <addr> <len>\n");
return -1;
}

if (argc > 1) {
char *memtype = argv[1];
AVRMEM *mem = avr_locate_mem(p, memtype);
if (mem == NULL) {
pmsg_error("(erase) %s memory type not defined for part %s\n", argv[1], p->desc);
return -1;
}
char *args[6] = {"write", memtype, "", "", "0xff", "..."};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not wrong, but tradition has it that argv lists are terminated by NULL, so just for superstition, I'd write char *args[] = {"write", memtype, "", "", "0xff", "...", NULL}; Admittedly, it's unlikely that the write code ever looks for the terminating NULL.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your input, I wasn't aware of this so if I add a NULL to the args array, should I pass 6 or 7 as the argc number to cmd_write?

Copy link
Collaborator

Choose a reason for hiding this comment

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

argc must be 6 (no change)

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://en.cppreference.com/w/cpp/language/main_function

argc - Non-negative value representing the number of arguments passed to the program from the environment in which the program is run.

argv - Pointer to the first element of an array of argc + 1 pointers, of which the last one is null and the previous ones, if any, point to null-terminated multibyte strings that represent the arguments passed to the program from the execution environment. If argv[0] is not a null pointer (or, equivalently, if argc > 0), it points to a string that represents the name used to invoke the program, or to an empty string.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had a look at the tokenize() function that provides the argc, argv pair for all the commands in the terminal. Indeed, that uses a terminating NULL (as convention has it). So better to change that line to char *args[] = {"write", memtype, "", "", "0xff", "...", NULL};

// erase <mem>
if (argc == 2) {
args[2] = "0";
args[3] = "-1";
}
// erase <mem> <addr> <len>
else {
args[2] = argv[2];
args[3] = argv[3];
}
return cmd_write(pgm, p, 6, args);
}

term_out("erasing chip ...\n");

// Erase chip and clear cache
Expand Down