Refactors and Comments #80

AEFeinstein · 2018-12-17T02:10:06Z

Shouldn't have any functional changes

Commented hpatimer.c Commented user_main.c

Cleaned up configurable struct Added #define for SAVE_LOAD_KEY Added custom command information in README.md

Added custom_commands.h Added functions to check for and get Added hpaRunning to hpatimer.c, removed hpa_running from user_main.c Made sure variables have initial values

cnlohr · 2018-12-17T07:28:30Z

uuhh... mind looking at those conflicts, @AEFeinstein ?

cnlohr · 2018-12-17T07:30:42Z

Aah just saw your note in #77 ... I will look into this soon.

cnlohr · 2019-05-22T18:58:53Z

@AEFeinstein Can you pull and refresh? Or should this be closed?

AEFeinstein · 2019-05-22T19:11:21Z

Created AEFeinstein#2, will manually merge in the near future.

AEFeinstein · 2019-05-22T19:19:33Z

@cnlohr, do you really need image.elf-0x00000.bin checked in?

AEFeinstein · 2019-05-22T21:17:58Z

Pulled and refreshed

cnlohr · 2019-05-23T23:27:28Z

@AEFeinstein Can you test colorchord desktop edition based on your changes here?

bbkiwi · 2019-05-24T05:36:34Z

I've just compared @cnlohr master on a nodemcu with @AEFeinstein PR. Both seem to work with no major problems. Compatible with my recent changes. Tried with and without SORT_NOTES and APPROXNORM. I like @AEFeinstein handling of the configuration parameters. I vote to accept the PR. I will be fixing a few bugs and have additional output options. It will easier if I work with on this new code.

I get no warnings compiling @cnlohr, but get these after the PR

~/82XX-projects/colorchord/embedded8266 (testAEF) $ make all
/home/bill/esp-open-sdk/xtensa-lx106-elf/bin/xtensa-lx106-elf-gcc -mlongcalls -Os -I/home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/include -Iesp82xx/include -I. -Iesp82xx/fwsrc -Iuser -I../embeddedcommon -DUSE_OPTIMIZE_PRINTF -DMFS_PAGE_OFFSET=532480 -DICACHE_FLASH -DDISABLE_CHARRX -DWS2812_FOUR_SAMPLE -DSOFTAP_CHANNEL=6 -DWEB_PORT=80 -DCOM_PORT=7777 -DBACKEND_PORT=7878 -DSLOWTICK_MS=50 -DVERSSTR='"Version: 8d82a-dev - Build Fri, May 24 2019, 17:34:29 +1200 with -DUSE_OPTIMIZE_PRINTF -DMFS_PAGE_OFFSET=532480 -DICACHE_FLASH -DDISABLE_CHARRX -DWS2812_FOUR_SAMPLE -DSOFTAP_CHANNEL=6 -DWEB_PORT=80 -DCOM_PORT=7777 -DBACKEND_PORT=7878 -DSLOWTICK_MS=50"' -g esp82xx/fwsrc/uart.c esp82xx/fwsrc/esp82xxutil.c esp82xx/fwsrc/flash_rewriter.c esp82xx/fwsrc/http.c esp82xx/fwsrc/commonservices.c esp82xx/fwsrc/http_custom.c esp82xx/fwsrc/mdns.c esp82xx/fwsrc/mfs.c user/custom_commands.c user/user_main.c user/ws2812_i2s.c user/hpatimer.c user/adc.c ../embeddedcommon/DFT32.c ../embeddedcommon/embeddednf.c ../embeddedcommon/embeddedout.c -flto -Wl,--relax -Wl,--gc-sections -nostdlib -L/home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib -Lesp82xx/libgcc_stripped.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libmain.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/liblwip.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libssl.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libupgrade.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libnet80211.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libwpa.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libphy.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/liblwip.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libcrypto.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libc.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libespnow.a /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib/libpp.a esp82xx/libgcc_stripped.a -T /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/ld/eagle.app.v6.ld -T /home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/ld/eagle.rom.addr.v6.ld -Wl,-Map,output.map -B/home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/lib -o image.elf
user/user_main.c: In function 'user_init':
user/user_main.c:270:2: warning: passing argument 1 of 'ets_timer_disarm' discards 'volatile' qualifier from pointer target type [enabled by default]
os_timer_disarm(&some_timer);
^
In file included from user/user_main.c:13:0:
/home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/include/osapi.h:66:6: note: expected 'struct ETSTimer *' but argument is of type 'volatile struct ETSTimer *'
void ets_timer_disarm(os_timer_t *ptimer);
^
user/user_main.c:271:2: warning: passing argument 1 of 'ets_timer_setfn' discards 'volatile' qualifier from pointer target type [enabled by default]
os_timer_setfn(&some_timer, (os_timer_func_t *)myTimer, NULL);
^
In file included from user/user_main.c:13:0:
/home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/include/osapi.h:67:6: note: expected 'struct ETSTimer *' but argument is of type 'volatile struct ETSTimer *'
void ets_timer_setfn(os_timer_t *ptimer, os_timer_func_t *pfunction, void *parg);
^
user/user_main.c:272:2: warning: passing argument 1 of 'ets_timer_arm_new' discards 'volatile' qualifier from pointer target type [enabled by default]
os_timer_arm(&some_timer, 100, 1);
^
In file included from user/user_main.c:13:0:
/home/bill/82XX-projects/colorchord/embedded8266/esp82xx/toolchain/esp_nonos_sdk/include/osapi.h:65:6: note: expected 'struct ETSTimer *' but argument is of type 'volatile struct ETSTimer *'
void ets_timer_arm_new(os_timer_t *ptimer, uint32_t time, bool repeat_flag, bool ms_flag);
^
PATH=/home/bill/esp-open-sdk/xtensa-lx106-elf/bin:/home/bill/bin:/home/bill/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/bill/esp-open-sdk/xtensa-lx106-elf/bin /home/bill/esp-open-sdk/esptool/esptool.py elf2image image.elf
esptool.py v1.2

Here is the space used:

with PR and SORT=1 and APPROXNORM=1:
section size addr
.data 1302 1073643520
.rodata 3724 1073644832
.bss 43064 1073648560
.irom0.text 219792 1075904512
.text 29716 1074790400
.comment 5520 0
.xtensa.info 56 0
Total 408605
with PR and SORT=0 and APPROXNORM=1
.text 29652 1074790400
Total 408289
with PR and SORT=0 and APPROXNORM=0
.text 29684 1074790400
Total 408682

AEFeinstein · 2019-05-28T12:35:12Z

I made some variables volatile because they were used inside and outside interrupts, but I guess the compiler isn't thrilled with that. I should be able to clean up those warnings with some casting later today.

AEFeinstein · 2019-05-29T00:03:18Z

Fixed the volatility thing. I'm getting this warning too:

user/ws2812_i2s.c:466:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  ~0b1000100010001000, ~0b1000100010001100, ~0b1000100011001000, ~0b1000100011001100,
  ^
user/ws2812_i2s.c:466:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:466:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:466:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:467:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  ~0b1000110010001000, ~0b1000110010001100, ~0b1000110011001000, ~0b1000110011001100,
  ^
user/ws2812_i2s.c:467:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:467:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:467:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:468:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  ~0b1100100010001000, ~0b1100100010001100, ~0b1100100011001000, ~0b1100100011001100,
  ^
user/ws2812_i2s.c:468:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:468:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:468:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:469:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  ~0b1100110010001000, ~0b1100110010001100, ~0b1100111011001000, ~0b1100110011001100,
  ^
user/ws2812_i2s.c:469:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:469:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
user/ws2812_i2s.c:469:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]

The code it's whining about seems perfectly reasonable though. 16 binary digits in a uint16_t looks good to me

static const uint16_t bitpatterns[16] = {
	~0b1000100010001000, ~0b1000100010001100, ~0b1000100011001000, ~0b1000100011001100,
	~0b1000110010001000, ~0b1000110010001100, ~0b1000110011001000, ~0b1000110011001100,
	~0b1100100010001000, ~0b1100100010001100, ~0b1100100011001000, ~0b1100100011001100,
	~0b1100110010001000, ~0b1100110010001100, ~0b1100111011001000, ~0b1100110011001100,
};

bbkiwi · 2019-05-29T03:13:50Z

I had noticed this warning when trying @cnlohr master which has #define LUXETRON at line 55 in ws2812_i2s.c which then defines INVERT. You won't get this if comment out the #define LUXETRON (which I did in my tests, (it should be in ccconfig.h or user.cfg). Maybe (???) the compiler views 0b1000100010001000 as negative, so warns when trying to bitwise invert it. Explicitly writing the inverted values should get rid of the warning.

static const uint16_t bitpatterns[16] = { 0b0111011101110111, ...

AEFeinstein · 2019-05-29T04:04:33Z

Or maybe appending a u? Just brainstorming while out on the town.

…

On Tue, May 28, 2019, 11:13 PM bbkiwi ***@***.***> wrote: I had noticed this warning when trying @cnlohr <https://github.com/cnlohr> master which has #define LUXETRON at line 55 in ws2812_i2s.c which then defines INVERT. You won't get this if comment out the #define LUXETRON (which I did in my tests, (it should be in ccconfig.h or user.cfg). Maybe the compiler views 0b1000100010001000 as negative, so warns when bitwise invert it??? Explictly writing the inversioned values should get rid of the warning. static const uint16_t bitpatterns[16] = { 0b0111011101110111, ... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#80?email_source=notifications&email_token=AABYODFPSHKRR3YIPZ5XISLPXXYHFA5CNFSM4GKWIZV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWOBBBQ#issuecomment-496767110>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABYODHOHEIF3VPOSC336ODPXXYHFANCNFSM4GKWIZVQ> .

bbkiwi · 2019-05-29T05:12:36Z

Appending u didn't work.
This get rid of the warnings
static const uint16_t bitpatterns[16] = { 0b0111011101110111, 0b0111011101110011, 0b0111011100110111, 0b0111011100110011, 0b0111001101110111, 0b0111001101110011, 0b0111001100110111, 0b0111001100110011, 0b0011011101110111, 0b0011011101110011, 0b0011011100110111, 0b0011011100110011, 0b0011001101110111, 0b0011001101110011, 0b0011000100110111, 0b0011001100110011, };

I forgot to include ccconfig.h in my last PR. So I've done that and made the above fix in a new PR.

AEFeinstein · 2019-05-29T12:26:55Z

Sounds good. Any other changes anyone wants me to make here?

…

On Wed, May 29, 2019, 1:12 AM bbkiwi ***@***.***> wrote: Appending u didn't work. This get rid of the warnings static const uint16_t bitpatterns[16] = { 0b0111011101110111, 0b0111011101110011, 0b0111011100110111, 0b0111011100110011, 0b0111001101110111, 0b0111001101110011, 0b0111001100110111, 0b0111001100110011, 0b0011011101110111, 0b0011011101110011, 0b0011011100110111, 0b0011011100110011, 0b0011001101110111, 0b0011001101110011, 0b0011000100110111, 0b0011001100110011, }; I forgot to include ccconfig.h in my last PR. So I done that and made the above fix in a new PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#80?email_source=notifications&email_token=AABYODGNO3OMPRUCNDRAF3TPXYGENA5CNFSM4GKWIZV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWOF44Q#issuecomment-496787058>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABYODBIPWL7WXXUIOG4T23PXYGENANCNFSM4GKWIZVQ> .

bbkiwi · 2019-05-29T22:03:07Z

PR #91 will make the changes, or you can fix it in your's and Charles can ignore mine.

AEFeinstein · 2019-05-29T22:53:21Z

#91 is already up, so lets just use that

cnlohr · 2019-09-27T05:48:35Z

Closing PR based on comments.

adam and others added 13 commits October 11, 2018 08:04

Cleanup to understand

045198b

Commented hpatimer.c Commented user_main.c

Added eclipse files

9a49ce3

Updated gitignore

939a047

Commented custom_commands.c

fdea631

Cleaned up configurable struct Added #define for SAVE_LOAD_KEY Added custom command information in README.md

Removed bins

26770db

Update README.md

7ec5ee0

Update README.md

c8e4757

Update README.md

9b41719

Update README.md

2c2dcfd

Update README.md

0e30589

More comments

374bf17

Merge branch 'master' of https://github.com/AEFeinstein/colorchord.git

12c0b95

Light refactoring

2783fa4

Added custom_commands.h Added functions to check for and get Added hpaRunning to hpatimer.c, removed hpa_running from user_main.c Made sure variables have initial values

AEFeinstein mentioned this pull request Dec 17, 2018

ColorChordCalliope #77

Open

AEFeinstein added 4 commits May 22, 2019 16:53

Attempted manual merge from cnlohr's master. May or may not have worked

3562bc4

Minor cleanup

722f93b

Merged from cnlohr's master

5f63977

Fixed compilation and some warnings

4d838ca

Fixed warning about volatility

c039973

cnlohr closed this Sep 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactors and Comments #80

Refactors and Comments #80

AEFeinstein commented Dec 17, 2018

cnlohr commented Dec 17, 2018

cnlohr commented Dec 17, 2018

cnlohr commented May 22, 2019

AEFeinstein commented May 22, 2019

AEFeinstein commented May 22, 2019

AEFeinstein commented May 22, 2019

cnlohr commented May 23, 2019

bbkiwi commented May 24, 2019 •

edited

Loading

AEFeinstein commented May 28, 2019

AEFeinstein commented May 29, 2019

bbkiwi commented May 29, 2019 •

edited

Loading

AEFeinstein commented May 29, 2019 via email

bbkiwi commented May 29, 2019 •

edited

Loading

AEFeinstein commented May 29, 2019 via email

bbkiwi commented May 29, 2019

AEFeinstein commented May 29, 2019

cnlohr commented Sep 27, 2019

Refactors and Comments #80

Refactors and Comments #80

Conversation

AEFeinstein commented Dec 17, 2018

cnlohr commented Dec 17, 2018

cnlohr commented Dec 17, 2018

cnlohr commented May 22, 2019

AEFeinstein commented May 22, 2019

AEFeinstein commented May 22, 2019

AEFeinstein commented May 22, 2019

cnlohr commented May 23, 2019

bbkiwi commented May 24, 2019 • edited Loading

AEFeinstein commented May 28, 2019

AEFeinstein commented May 29, 2019

bbkiwi commented May 29, 2019 • edited Loading

AEFeinstein commented May 29, 2019 via email

bbkiwi commented May 29, 2019 • edited Loading

AEFeinstein commented May 29, 2019 via email

bbkiwi commented May 29, 2019

AEFeinstein commented May 29, 2019

cnlohr commented Sep 27, 2019

bbkiwi commented May 24, 2019 •

edited

Loading

bbkiwi commented May 29, 2019 •

edited

Loading

bbkiwi commented May 29, 2019 •

edited

Loading