davidmroth / stm32flash

Automatically exported from code.google.com/p/stm32flash
0 stars 0 forks source link

Patches for merging brunch #39

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Here is a list of patches with my improvements of stm32flash

1) fix memory write procedure in case when we write file to default location.
2) Fix memory erase proc (Extended Erase)

Original issue reported on code.google.com by alat...@list.ru on 18 Dec 2012 at 3:54

Attachments:

GoogleCodeExporter commented 9 years ago
Additional patches
3) Usability: CLI redesign
4) Code cleanup (not functional changes)
5) Changed types of flag variables
6) Allow to work with RAM (not well tested yet)
7) Serial communication improvements (try to reconnect if cannot INIT and some 
other)

Original comment by alat...@list.ru on 19 Dec 2012 at 10:00

Attachments:

GoogleCodeExporter commented 9 years ago
Few correction of 002

Original comment by alat...@list.ru on 22 Dec 2012 at 7:12

Attachments:

GoogleCodeExporter commented 9 years ago
Some changes in read/write protect/unprotect
8) remove unneeded bytes
9) make function names more standardised

Original comment by alat...@list.ru on 27 Dec 2012 at 7:52

Attachments:

GoogleCodeExporter commented 9 years ago
Small bugfix

Original comment by alat...@list.ru on 29 Dec 2012 at 5:06

Attachments:

GoogleCodeExporter commented 9 years ago
Another bugfixed and usage improvements

Original comment by alat...@list.ru on 29 Dec 2012 at 6:36

Attachments:

GoogleCodeExporter commented 9 years ago
Looks like author and Tormod Volden are not interested in the project now. So 
I'm create new fork - http://developer.berlios.de/projects/stmflasher/ 
It is welcome any interested developers who want to help with improve this 
program.

Original comment by alat...@list.ru on 24 Feb 2013 at 7:59

GoogleCodeExporter commented 9 years ago
I am still interested. But I haven't had time to review properly all your 
patches. They are many and changes a lot of code. Ideally more people would 
participate in code reviews. Maybe we should have a mailing list, I think that 
is better for code review than a bug tracker.

There are also some known bugs in my current "merging" branch that I was hoping 
the original patch submitters would fix up themselves, and I wanted to have 
that fixed before applying yours. But maybe your patches fix those issues 
anyway. It would be easier for the maintainer (still hoping he will come back) 
to pull in clean patches instead of a series of broken ones plus fix-ups.

Original comment by lists.to...@gmail.com on 24 Feb 2013 at 9:32

GoogleCodeExporter commented 9 years ago
001-addreses_calculation_fix.patch: (I just read quickly through it)
The patch has no explanation and typos in the title
line 45: typo
line 83: left old code
line 101: left personal comment

Original comment by lists.to...@gmail.com on 24 Feb 2013 at 9:43

GoogleCodeExporter commented 9 years ago
003-Usability-CLI-redesign.patch:
Please explain the difference between showinfo and verbose. Also, wasn't the 
idea that diag could be redirected to for instance stderr or a null device in 
case of a non-verbose run?
line 186: Are you renaming the options? That would be a major change and should 
be in a separate commit. At least be documented in the patch header.

For these kind of programs I like them to be verbose by default. When people 
have trouble you will not have to ask them to use verbose options. Ideally it 
would also print out some version information every time...

Original comment by lists.to...@gmail.com on 24 Feb 2013 at 9:56

GoogleCodeExporter commented 9 years ago
004-Code-cleanup.patch:
Code comments are great, and it is wonderful to have these changes in a 
separate commit. However, shouldn't we standardize on old-school */...*/ 
comment style?

Original comment by lists.to...@gmail.com on 24 Feb 2013 at 10:04

GoogleCodeExporter commented 9 years ago
005-Changed-types-of-flag-variables.patch:
What is the advantage of this?

006-Allow-to-work-with-RAM.patch:
line 89: belongs to some other patch
line 130, "cannot use pages with RAM"

007-Serial-communication.patch
line 8: explain which one you are actually changing
The patch can safely be split up into independent patches.

008-remove-wrong-cmd-from-rw_prot_unprot.patch:
line 4 belongs in patch description, patch file name should be in patch title

009-Rename-readprot-to-rprot.patch: why?

010-Bugfix-wrong-condition.patch:
If this was written wrong in 003-Usability-CLI-redesign.patch you should fix up 
that patch instead.

011-Fix-pages-addessing.patch:
Same, you should rebase and fix up the older patches instead.

012-some-usage-and-output-changes.patch:
lines 28-38 belongs in 006-Allow-to-work-with-RAM.patch
You can also use "<device>" in the help text.

Original comment by lists.to...@gmail.com on 24 Feb 2013 at 10:30

GoogleCodeExporter commented 9 years ago
Sorry, but some of this patches are already obsolete and rewritten in my trunk 
version.

>>There are also some known bugs in my current "merging" branch that I was 
hoping the original patch submitters would fix up themselves, and I wanted to 
have that fixed before applying yours.

My patches fixes some of those bugs

>> still hoping he will come back

I`m not really hope this now, so I want to create full-functional fork with own 
bug-tracker/feather request system and etc. But mailing list is good idea too.

>> Please explain the difference between showinfo and verbose.

I thing there is no need to show MCU info any time when I want to read/write 
firmware, so I add separate flag for show MCU info. Verbose is mode, where 
shown other commonly unneeded information (Serial port config, working space 
addresses).
In my trunk this code was some rewritten and now I have silent mode (-V 0), 
where all diag output disabled with "if(...)". But redirecting dial to null 
sound interesting. By default I print some basic info and all errors. I don`t 
thing that is needed to show version info every time.

>> However, shouldn't we standardize on old-school */...*/ comment style?

I thing you right, I will try to use only c-style comments.

>> What is the advantage of this?

Only for more clean code understanding.

>> line 130, "cannot use pages with RAM"

Already fixed

>> line 8: explain which one you are actually changing

In original stm32flash programm stops if got NACK from INIT and user need to 
manually restart program with -C. I'm add auto resume feature.

>> 009-Rename-readprot-to-rprot.patch: why?

For standardization with runprot and wunprot.

>> You can also use "<device>" in the help text.

I think in examples section it is more clean to use real port name.

About patches style I don't have much time too, I simply want to have working 
version of program. I understand that I should better spit and maintain 
patches, but it is hard to do when they all integrated in source code. Sorry.

Original comment by alat...@list.ru on 24 Feb 2013 at 12:53