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

Arduino boards with ESP8266 #3121

Merged
merged 4 commits into from
Apr 11, 2017

Conversation

sergiotomasello
Copy link
Contributor

@sergiotomasello sergiotomasello commented Apr 6, 2017

This PR introduces a new board called Arduino with three models:

  • Star OTTO
  • Primo
  • UNO WiFi

@codecov-io
Copy link

codecov-io commented Apr 6, 2017

Codecov Report

Merging #3121 into master will decrease coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3121      +/-   ##
=========================================
- Coverage   27.82%   27.6%   -0.23%     
=========================================
  Files          20      20              
  Lines        3626    3655      +29     
  Branches      656     678      +22     
=========================================
  Hits         1009    1009              
- Misses       2441    2468      +27     
- Partials      176     178       +2
Impacted Files Coverage Δ
cores/esp8266/spiffs_api.h 44.37% <0%> (-0.33%) ⬇️
cores/esp8266/spiffs/spiffs_nucleus.c 41.78% <0%> (-0.31%) ⬇️
cores/esp8266/spiffs/spiffs_hydrogen.c 38.06% <0%> (-0.28%) ⬇️
cores/esp8266/spiffs/spiffs_gc.c 2.57% <0%> (-0.05%) ⬇️
cores/esp8266/WString.cpp 15.13% <0%> (-0.04%) ⬇️
cores/esp8266/Print.cpp 5.03% <0%> (-0.04%) ⬇️
cores/esp8266/core_esp8266_noniso.c 0% <0%> (ø) ⬆️
cores/esp8266/spiffs/spiffs_check.c 0% <0%> (ø) ⬆️
cores/esp8266/Stream.cpp 17.42% <0%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a01638f...663244e. Read the comment docs.

Copy link
Member

@igrr igrr left a comment

Choose a reason for hiding this comment

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

Changes look mostly good, just a small suggestion about the way f_crystal is defined.

@@ -88,7 +88,11 @@ static const uint8_t ICACHE_FLASH_ATTR phy_init_data[128] =
// 0: 40MHz
// 1: 26MHz
// 2: 24MHz
#ifdef F_CRYSTAL
Copy link
Member

@igrr igrr Apr 6, 2017

Choose a reason for hiding this comment

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

Let me suggest an approach which will not require f_crystal value to be added to all the boards... Since 26MHz is pretty much the default for the ESP8266 based designs, we only need to add f_crystal=40000000 to the boards which use 40 MHz crystal.

Then you can have something along these lines:

#if F_CRYSTAL == 40000000
    [48] = 0,
#else
    [48] = 1,
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I can change it, but my approach allows users to use different f_crystal values even for the esp8266 generic board

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, let's keep this menu for generic. Other boards can use 26MHz via silent default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eheh, I just pushed a new commit without menu... let me know if you want me to put it back inside :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm going to push a new (squashed) commit

boards.txt Outdated
@@ -1397,6 +1430,7 @@ espino.serial.disableRTS=true

espino.build.mcu=esp8266
espino.build.f_cpu=80000000L
espino.build.f_crystal=1
Copy link
Member

Choose a reason for hiding this comment

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

suggest making f_crystal value similar to f_cpu, by expressing it in Hz.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

arduino-esp8266.name=Arduino

arduino-esp8266.upload.tool=esptool
arduino-esp8266.upload.speed=9600
Copy link
Member

Choose a reason for hiding this comment

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

Is the default upload speed intentionally set that low?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, because into uno wifi dev. ed. the maximum speed between ATmega328P and esp8266 is 9600, for the other boards the speed can be higher.

@sergiotomasello
Copy link
Contributor Author

@igrr when you believe that it can be merged and when you believe that version 2.4.0 will be released for the Arduino IDE Board Manager?

@igrr
Copy link
Member

igrr commented Apr 10, 2017

Seems like the build has failed, please check the syntax:
https://travis-ci.org/esp8266/Arduino/builds/220486369#L837

I can merge this as soon as build passes.
W.r.t. releasing 2.4.0, can't promise anything, there is a lot of functionality currently broken in master.

boards.txt Outdated
generic.build.board=ESP8266_ESP01
generic.build.core=esp8266
generic.build.variant=generic
generic.build.flash_mode=qio
generic.build.spiffs_pagesize=256
generic.build.debug_port=
generic.build.debug_level=
generic.build.extra_flags=-DF_CRYSTAL={build.f_crystal}
Copy link
Member

Choose a reason for hiding this comment

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

seems like this expression is not getting expanded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arduino IDE expands it before compiling, into console I get the correct value: -DF_CRYSTAL=24000000

Compiling sketch...
"/Users/sergio/Library/Arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/1.20.0-26-gb404fb9-2/bin/xtensa-lx106-elf-g++" -D__ets__ -DICACHE_FLASH -U__STRICT_ANSI__ "-I/Users/sergio/Library/Arduino15/packages/esp8266/hardware/esp8266/2.3.0/tools/sdk/include" "-I/Users/sergio/Library/Arduino15/packages/esp8266/hardware/esp8266/2.3.0/tools/sdk/lwip/include" "-I/Users/sergio/Library/Arduino15/packages/esp8266/hardware/esp8266/2.3.0/tools/sdk/libc/xtensa-lx106-elf/include" "-I/var/folders/k4/q1bl6yfs2rv0n006w3nh0rk40000gn/T/arduino_build_308216/core" -c -w -Os -g -mlongcalls -mtext-section-literals -fno-exceptions -fno-rtti -falign-functions=4 -std=c++11 -MMD -ffunction-sections -fdata-sections -DF_CPU=80000000L -DLWIP_OPEN_SRC   -DARDUINO=10801 -DARDUINO_ESP8266_ESP01 -DARDUINO_ARCH_ESP8266 -DARDUINO_BOARD="ESP8266_ESP01"  -DF_CRYSTAL=24000000 "-I/Users/sergio/Library/Arduino15/packages/esp8266/hardware/esp8266/2.3.0/cores/esp8266" "-I/Users/sergio/Library/Arduino15/packages/esp8266/hardware/esp8266/2.3.0/variants/generic" "/var/folders/k4/q1bl6yfs2rv0n006w3nh0rk40000gn/T/arduino_build_308216/sketch/sketch_apr11a.ino.cpp" -o "/var/folders/k4/q1bl6yfs2rv0n006w3nh0rk40000gn/T/arduino_build_308216/sketch/sketch_apr11a.ino.cpp.o"

This kind of expression is needed if you want to keep the Crystal Freq. Menu to allow users to select crystal frequency by themselves.

For the same reason I should also change this:

arduino-esp8266.build.extra_flags=-DF_CRYSTAL={build.f_crystal}

Let me know how to proceed and move forward ;)

Copy link
Member

Choose a reason for hiding this comment

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

I suppose that Travis build is failing because I'm using an older Arduino IDE version there?

Anyway, I'm even okay with having f_crystal=26000000 specified for every board. Hope this can fix the build issue.

Copy link
Contributor Author

@sergiotomasello sergiotomasello Apr 11, 2017

Choose a reason for hiding this comment

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

probably... which version are you using?

If you are agree I proceed in this way:

  • change #if statement in core_esp8266_phy.c as you suggest the first time
    #if F_CRYSTAL == 40000000
      [48] = 0,
    #else
      [48] = 1,
    #endif
  • remove Crystal Freq. menu from boards.txt
  • update arduino-esp8266 entries to prevent Travis build failure

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, actually I'm using nightly:
https://github.com/esp8266/Arduino/blob/master/.travis.yml#L23

Agree with your suggestion, please go ahead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done! build success 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants