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

Guard CommandExecutor and CommandHandler #1014

Merged
merged 4 commits into from
Mar 8, 2017

Conversation

ADiea
Copy link
Contributor

@ADiea ADiea commented Mar 7, 2017

This functionality is sometimes not used but because it is present in places like websock and http server it occupies RAM and heap

Without CommandExecutor, compiling sample HttpServer_WebSockets

#Memory / Section info:
------------------------------------------------------------------------------
   Section|                   Description| Start (hex)|   End (hex)|Used space
------------------------------------------------------------------------------
      data|        Initialized Data (RAM)|    3FFE8000|    3FFE8384|     900
    rodata|           ReadOnly Data (RAM)|    3FFE8390|    3FFEA264|    7892 (-956)
       bss|      Uninitialized Data (RAM)|    3FFEA268|    3FFF1100|   28312 (-144)
      text|            Cached Code (IRAM)|    40100000|    40105E46|   24134
irom0_text|           Uncached Code (SPI)|    4020A000|    40254163|  303459 (-3247)
Total Used RAM : 37104
Free RAM : 44816
Free IRam : 8652
------------------------------------------------------------------------------

@slaff slaff added this to the 3.1.2 milestone Mar 7, 2017
Copy link
Contributor

@slaff slaff left a comment

Choose a reason for hiding this comment

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

@ADiea When I export DISABLE_CMD_EXEC=1 and the compile the samples some of them fail (Arducam, CommandProcessing_Debug and Telnet_TCPServer_TCPClient for example).

Please, make sure that they compile. Steps to reproduce

cd $SMING_HOME
export DISABLE_CMD_EXEC=1
make dist-clean
make samples

@anakod
Copy link
Member

anakod commented Mar 8, 2017

I prefer use full names to keep things clear, it shouldn't be problem to write it :)

DISABLE_COMMAND_EXECUTOR or DISABLE_CMD_EXECUTOR

@anakod
Copy link
Member

anakod commented Mar 8, 2017

Sorry! Have checked current Makefile code, if we already have "ENABLE_SSL" than we should add new flags in same manner, I think: ENABLE_XXX.
So ENABLE_COMMAND_EXECUTOR=0 or ENABLE_CMD_EXECUTOR=0

@@ -188,7 +191,7 @@ EXTRA_INCDIR += $(SMING_HOME)/include $(SMING_HOME)/ $(LWIP_INCDIR) $(SMING_HOME
USER_LIBDIR = $(SMING_HOME)/compiler/lib/

# compiler flags using during compilation of source files
CFLAGS = -Wpointer-arith -Wundef -Werror -Wl,-EL -nostdlib -mlongcalls -mtext-section-literals -finline-functions -fdata-sections -ffunction-sections -D__ets__ -DICACHE_FLASH -DARDUINO=106 -DCOM_SPEED_SERIAL=$(COM_SPEED_SERIAL) $(USER_CFLAGS)
CFLAGS = -Wpointer-arith -Wundef -Werror -Wl,-EL -nostdlib -mlongcalls -mtext-section-literals -finline-functions -fdata-sections -ffunction-sections -D__ets__ -DICACHE_FLASH -DARDUINO=106 -DCOM_SPEED_SERIAL=$(COM_SPEED_SERIAL) $(USER_CFLAGS) -DDISABLE_CMD_EXEC=$(DISABLE_CMD_EXEC)
Copy link
Contributor

Choose a reason for hiding this comment

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

The same directive should be also in the Makefile-project.mk file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically this is only used for sming building so app should not care for this flag. But will add it anyway for completion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then just remove it from here. Rename also the directive as anakod suggested: ENABLE_COMMAND_EXECUTOR and take care to change the preprocessor if-conditions accordingly.

@slaff slaff removed the 3 - Review label Mar 8, 2017
@slaff slaff merged commit 7dec45f into SmingHub:develop Mar 8, 2017
@ADiea ADiea mentioned this pull request Mar 8, 2017
21 tasks
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