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.

Sign up for GitHub

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

Jump to bottom

boards.txt generator #3722

Merged
merged 2 commits into from
Jan 8, 2018
Merged

boards.txt generator #3722

merged 2 commits into from
Jan 8, 2018

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Oct 15, 2017 •
edited
Loading

To quickly try it in a separate branch:
git fetch origin pull/3722/head:pr-3722 && git checkout pr-3722; git branch
tools/boards.txt.py --help
tools/boards.txt.py > boards.txt
and restart arduino.

latest boards.txt - without debug option NULL nor generic boards' LED_BUILTIN_LED
2.4.0rc2 boards.txt before the generator

TODO: #3982

@d-a-v d-a-v mentioned this pull request Oct 15, 2017
Closed
@davisonja
Copy link

Why are entries in 'opts' in a different format (key/value) to the entries in 'macros' (list)?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 16, 2017

Python's dicts do not preserve order (like map:: in c++).
I had dicts everywhere but I switched later to lists were I needed to preserve order.
Should I convert 'opts' format to lists ?

@davisonja
Copy link

davisonja commented Oct 16, 2017 via email

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 16, 2017

By 'preserve order' I was meaning the order coded in the source code which reflects the order in arduino gui's menu. I just learned that python>=3.6 preserves the source order. Or we could use python's OrderedDict (the syntax changes).
I was wrong saying that we could easily use list everywhere, because I use 'opts' dict to make a search (line 656) to allows 'opts' to override a macro entry.

if [...] (keyval[0] in board['opts'])

This leaves:

  • use from collections import OrderedDict
  • force the use of python 3.6 :-)
  • stick with mix of list and dict
  • something else I'm not aware of

If you need the search facility of dicts, I guess I should try and use OrderedDict ?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 16, 2017

There are dicts instead of lists now.

davisonja
davisonja reviewed Oct 16, 2017
View reviewed changes
tools/boards.txt.py Outdated
if not ('opts' in board) or not (keyval[0] in board['opts']):
print short + keyval[0] + '=' + keyval[1]
for key in macros[block]:
if not ('opts' in board) or not (str(key) in board['opts']):
Copy link

Choose a reason for hiding this comment

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

I also think the dict makes this line clearer with 'key' instead of 'keyval[0]' :)

@davisonja
Copy link

It occurred to me a little later that lists would prevent the lookup in line 656 too :)

Using an OrderedDict (as you've done) is, I think, the simplest.

The other relatively easy approach (though with the layers here I wouldn't recommend it!) is to use dicts to store the info, and lists to index with order. So, for determining the order of the boards, instead of

boards = dict(...)
for board in boards:
    process(board)

you instead have

boards = dict(...)
menu_order = ['board1', 'board2', 'board3', ...]
for menu_entry in menu_order:
    process(boards[menu_entry])

@igrr
Copy link
Member

igrr commented Oct 16, 2017

Not intending to start a holy war here, but since this is Python, can we switch indentation to spaces?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 16, 2017

I hope I understood everything well with the last commit.

@devyte devyte mentioned this pull request Oct 25, 2017
Closed
This was referenced Nov 1, 2017
Closed
Merged
@davisonja
Copy link

My question is about the 'helper function 'flash_size' - I'm wondering what advantage this provides over specifying the flash info in the same way as for the other board info (in dict()s)?
I like the approach of having all the data in data structures, and processing code that deals with it. In addition to clearly separating the (changeable) data from the code, it lends itself to having an inverse script that can generate the data structures from an existing boards.txt (and you can then plug in the data structures).
I'm not suggesting that the existing script is hard to follow, but it feels more involved and intertwined than is really necessary for the fairly simple source data we're dealing with - which leads me to my original question as I wonder what I'm missing :)

I'm also not a fan of positional arguments (particularly when mixed with keyword arguments) and would vote for using keyword arguments across the board.

@igrr
Copy link
Member

igrr commented Nov 3, 2017

I vaguely remember writing something about 'helper function flash_size' some time ago, but can't even find my own comment anymore. Anyway, current changeset looks good to me!

d-a-v referenced this pull request Nov 5, 2017
@d-a-v @igrr
lwip2 integration
32f6826
@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 5, 2017

This question has raised about the default serial upload speed. In the script I put 115200 by default for all boards, because

  • I thought it was like that in the old boards.txt
  • It is the most compatible value (I experienced problems with 921k on macOS and wemos, 460k was ok)

But after checking, it appears that at least the Wemos D1 R2/mini had 921k by default.
So should we use 921k by default for all boards ?

@davisonja
Copy link

davisonja commented Nov 5, 2017 via email

@igrr
Copy link
Member

igrr commented Nov 6, 2017 via email

@davisonja
Copy link

davisonja commented Nov 6, 2017 via email

@igrr
Copy link
Member

igrr commented Nov 6, 2017 via email

@davisonja
Copy link

davisonja commented Nov 6, 2017 via email

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 6, 2017 •
edited
Loading

and what about writing by default a "secure reliable 115200" config, with command line options for users willing to regenerate with other defaults ?

@davisonja
Copy link

davisonja commented Nov 6, 2017 via email

@davisonja
Copy link

Just checked, it is "boards.local.txt", from revisions.txt:

ARDUINO 1.6.6 - 2015.11.03
[...]

  • Added boards.local.txt support: like platform.local.txt, allows to enrich a boards.txt definition without modifying the original file. Thanks @Wackerbarth

@d-a-v d-a-v mentioned this pull request Nov 8, 2017
Closed
@devyte
Copy link
Collaborator

devyte commented Nov 15, 2017

@davisonja @d-a-v what's the status on this?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 15, 2017

Current output is in master's board.txt with so far no issues.
Improvements will be done about defaults values (as asked here and here)
@davisonja may be working on it for autogeneration of ldscripts.
I can try and automate all_flash_size().
@igrr what are the priorities ?

@davisonja
Copy link

I am in theory, but have been a bit pressed for time this week. Will get onto it this weekend properly.
What automation are you thinking of for all_flash_size()?

I think should have an additional function for generating the ldscripts (possibly refactor so we have several functions that output to the file system, one for ldscripts and one for boards.txt itself?) to keep the flow clean.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Dec 12, 2017

@davisonja @igrr Unless you think it is unwise, I plan to produce doc/boards.rst with the generator.

@devyte
Copy link
Collaborator

devyte commented Dec 12, 2017

@d-a-v I was actually going to suggest the same thing. Not generating makes for doc maintenance, and additional issues.

@Pablo2048
Copy link

I'm trying to implement 4M/2M SPIFFS variant an I ended with attached linker script and following modification to boards.txt:

generic.menu.FlashSize.4M2M=4M (2M SPIFFS)
generic.menu.FlashSize.4M2M.build.flash_size=4M
generic.menu.FlashSize.4M2M.build.flash_ld=eagle.flash.4m2m.ld
generic.menu.FlashSize.4M2M.build.spiffs_start=0x200000
generic.menu.FlashSize.4M2M.build.spiffs_end=0x3FB000
generic.menu.FlashSize.4M2M.build.spiffs_blocksize=8192
generic.menu.FlashSize.4M2M.build.spiffs_pagesize=256
generic.menu.FlashSize.4M2M.upload.maximum_size=1044464

So I'm asking to guide how to add this into the generator?

eagle.flash.4m2m.zip

@d-a-v
Copy link
Collaborator Author

d-a-v commented Dec 14, 2017 •
edited
Loading

@Pablo2048
A new line there is needed.
Size is 0x3FB000-0x200000
The ldscript would be autogenerated with proper option

@Pablo2048
Copy link

Pablo2048 commented Dec 14, 2017 •
edited
Loading

@d-a-v thank You. And how to handle the linker script?
Oops - sorry, I write this faster than understood what You've write...

@Pablo2048
Copy link

@d-a-v looks like this line did the trick
f4m.update( flash_size( '4M', '4M2M', 'eagle.flash.4m2m.ld', '2M SPIFFS', 1044464, 0x200000, 0x1FB000, 8192))
so I'll do heavy testing on this and report back...

@d-a-v
Copy link
Collaborator Author

d-a-v commented Dec 14, 2017 •
edited
Loading

with option --ld or --ldgen

$ ./tools/boards.txt.py 

boards.txt generator for esp8266/Arduino

usage: ./tools/boards.txt.py [options]

        -h, --help
        --lwip                  - preferred default lwIP version (default 2)
        --led                   - preferred default builtin led for generic boards (default 2)
        --board b               - board to modify:
                --speed s       - change default serial speed
        --premerge              - no NULL debug option, no led menu
        --customspeed s         - new serial speed for all boards

        mandatory option (at least one):

        --boards                - show boards.txt
        --boardsgen             - replace boards.txt
        --ld                    - show ldscripts
        --ldgen                 - replace ldscripts
        --package               - show package
        --packagegen            - replace board:[] in package

available serial speed options (kbps): 9 57 115 230 256 460 512 921 
available board names: generic(115k) esp8285(115k) espduino(115k) huzzah(115k) espresso_lite_v1(115k) espresso_lite_v2(115k) phoenix_v1(115k) phoenix_v2(115k) nodemcu(115k) nodemcuv2(115k) modwifi(115k) thing(115k) thingdev(115k) esp210(57k) d1_mini(921k) d1_mini_pro(921k) d1_mini_lite(921k) d1(921k) espino(115k) espinotee(115k) wifinfo(115k) arduino-esp8266(115k) gen4iod(115k) oak(921k)

@d-a-v d-a-v mentioned this pull request Dec 16, 2017
Closed
@d-a-v d-a-v mentioned this pull request Dec 28, 2017
Closed
@d-a-v d-a-v mentioned this pull request Jan 2, 2018
Closed
@d-a-v d-a-v force-pushed the boards branch 2 times, most recently from f6a8429 to e6898b6 Compare January 4, 2018 02:53
@d-a-v
Copy link
Collaborator Author

d-a-v commented Jan 4, 2018

All commit are squashed into a single one.

commit log:

boards.txt generator
+ generates boards.rst
+ replace boards in package.json
+ generate ldscripts
+ new debug option: OOM
+ led menu for generic board

Only boards.txt is updated (OOM debug option, led menu for the generic board and I included #4083)
I hope you like the usefulness of OOM debug option as much as I do :)

This needs review (the script itself only if you have too much time).
The OOM debug option spreads accross multiple files but is harmless when not enabled.

@devyte devyte added this to the 2.5.0 milestone Jan 7, 2018
@d-a-v d-a-v assigned devyte and unassigned devyte Jan 8, 2018
devyte
devyte reviewed Jan 8, 2018
View reviewed changes
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

My only comment is whether the const char strings passed to os_printf() can be put in flash. There are several instances.

cores/esp8266/heap.c Outdated
{
void* ret = umm_malloc(s);
if (!ret)
os_printf(":oom(%d)@?\n", (int)s);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the strings like this one ":oom()blah" be put in flash?

@d-a-v
boards.txt generator
2e17ea8 
+ generates boards.rst
+ generate and replace boards section in package.json
+ generate ldscripts
+ new debug option: OOM
+ new led menu for generic board
@d-a-v
Copy link
Collaborator Author

d-a-v commented Jan 8, 2018

all these strings are now moved to flash

:oom(46200)@/my/path/to/realloc.ino:31
:oom(5)@user_interface.c:111

@devyte devyte merged commit 28253c5 into esp8266:master Jan 8, 2018
@devyte
Copy link
Collaborator

devyte commented Jan 8, 2018

I think this has been reviewed enough. If there are further things to change, let's discuss it in a new PR.

@d-a-v d-a-v deleted the boards branch January 8, 2018 14:10
@d-a-v d-a-v mentioned this pull request Feb 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davisonja davisonja davisonja left review comments

@igrr igrr igrr left review comments

@devyte devyte devyte approved these changes

Assignees

@d-a-v d-a-v

Labels
component: boards
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.
6 participants
@d-a-v @davisonja @igrr @devyte @whyameye @Pablo2048