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

[refactoring] Configurations are in the sources and not in a separate file #237

Closed
rewolff opened this issue May 27, 2014 · 12 comments · Fixed by #1129
Closed

[refactoring] Configurations are in the sources and not in a separate file #237

rewolff opened this issue May 27, 2014 · 12 comments · Fixed by #1129

Comments

@rewolff
Copy link
Contributor

rewolff commented May 27, 2014

Things like the memory map, and the stuff to send to GDB are included in the source. This means that when a new chip comes out, you have to edit the source to be able to work with it. I had that yesterday when I needed to patch the source for my 030F chip (and I didn't notice until later that "git pull" would've worked).
Issue #218 is the same: new chip, slightly different -> needs source code patch.

Suggestion:

  • Like AVRDUDE move the chip descriptions to a config file. Actually, having a config directory is better: Then it is much easier for people to share "Here, take my STM32F429 definition file!".
  • optional: If an unknown chip is found, offer to update the "database" of known chips over the internet. Have an "version" identifier in the chip-description-files that gets updated when a higher version of the source becomes necessary. (but know that the chip matches before checking the version: "This chip requires an update of the program"
@xor-gate xor-gate added this to the Unplanned (Contributions Welcome) milestone May 21, 2016
@Nightwalker-87 Nightwalker-87 modified the milestones: Unplanned (Contributions Welcome), Next Feb 19, 2020
@Nightwalker-87 Nightwalker-87 modified the milestones: General, v1.6.2 Feb 26, 2020
@Nightwalker-87 Nightwalker-87 modified the milestones: v1.6.2, v1.6.1 Mar 17, 2020
@Nightwalker-87
Copy link
Member

I agree on that. This is not a good practice.

@Nightwalker-87
Copy link
Member

Here is an example I just bumped into: src/chipid.c.
In my opinion all content of static const struct stlink_chipid_params devices[] should go in include/stlink/chipid.h.

@rewolff
Copy link
Contributor Author

rewolff commented Mar 24, 2020

No. I think "chipid.c" should remain having the same interface towards the rest of "stlink".
However, the content of the structure should be read from files.

I've been slowly thinking about this for a few days now. AVRDUDE has this one big file with all the chips and stuff in it. Not good. What we should do is to make a directory with a STM32F0x2.chip file in there that describes the F0x2 chips. Generally just one distinguishable chip per file. So suppose that a new STM32F826 comes out. Now people can just share the new description file and put it in the right place and be done with it. Editing the avrdude.conf file is a hassle.
With the ".chip" extension, the chips are one class of files. If stlink suite starts supporting more hardware, like avrdude, each programmer would get a .programmer file. Stuff like that.

edit: Oh. I seem to have retyped some of my opening-post here...

@Nightwalker-87
Copy link
Member

No. I think "chipid.c" should remain having the same interface towards the rest of "stlink".
However, the content of the structure should be read from files.

I did not mean that we should get rid of chipid.c. However your idea to read it from a file for each chip sounds good to me.

@Nightwalker-87 Nightwalker-87 changed the title Configuration things are in the sources instead of in a configuration file. [code-structure] Configuration things are in the sources instead of in a configuration file. Mar 28, 2020
@Nightwalker-87 Nightwalker-87 modified the milestones: v1.6.1, v1.6.2 Apr 8, 2020
@Nightwalker-87 Nightwalker-87 modified the milestones: v1.6.2, v1.6.3 Dec 25, 2020
@Nightwalker-87
Copy link
Member

@Ant-ON Maybe your thoughts to refactor src/common.c would fit in here.
I just noticed a lot of dead code and rubbish in there also, but haven't changed anything yet ...

@Ant-ON
Copy link
Collaborator

Ant-ON commented Apr 16, 2021

@Nightwalker-87 I've seen it, but there is no plan to implement it yet. I would only like to expand the list of chips by add the missing ones.

@Nightwalker-87
Copy link
Member

Yeah, that's just fine. I expect things to calm down a bit anyway once the release is ready...

@rewolff
Copy link
Contributor Author

rewolff commented Apr 16, 2021

preliminary code written, pull request created, "for discussion".

@Nightwalker-87
Copy link
Member

@rewolff We are currently waiting for another PR (#1145) to go into the testing branch first where we'll continue with this.
You may then resume with your proposal to address this topic.

@Nightwalker-87
Copy link
Member

Nightwalker-87 commented May 31, 2021

Ok, merged and preserved the defines recently aligned in format.
However I took note of that MCU descriptions (inline-comments) and defines in chipid.c do not match always.
One may address this during sanitation and comparison of chip-id configs and the old datasets.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.