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

Moves WiFi tester from system-part2 into user module #1378

Merged
merged 2 commits into from
Nov 21, 2017

Conversation

avtolstoy
Copy link
Member

Problem

We are currently running low on flash space in system-part2. As WiFiTester code is mainly used during manufacturing and testing, we can move it out of system firmware into a user module and enable it conditionally.

Solution

Moves WiFiTester into the user module 😄

Sizes comparisons:
0.7.0-rc.3:

   text	   data	    bss	    dec	    hex	filename
 261108	    156	    268	 261532	  3fd9c	../../../build/target/system-part1/platform-6-m/system-part1.elf
   text	   data	    bss	    dec	    hex	filename
 252972	   2260	  38484	 293716	  47b54	../../../build/target/system-part2/platform-6-m/system-part2.elf
   text	   data	    bss	    dec	    hex	filename
   6468	    156	   1400	   8024	   1f58	../../../build/target/user-part/platform-6-m/tinker.elf

This branch

   text	   data	    bss	    dec	    hex	filename
 261108	    156	    268	 261532	  3fd9c	../../../build/target/system-part1/platform-6-m/system-part1.elf
   text	   data	    bss	    dec	    hex	filename
 249788	   2260	  38508	 290556	  46efc	../../../build/target/system-part2/platform-6-m/system-part2.elf
   text	   data	    bss	    dec	    hex	filename
  10900	    156	   1400	  12456	   30a8	../../../build/target/user-part/platform-6-m/tinker.elf

Steps to Test

  • tinker
  • app/listen_mode test
  • wifitester app

Example App

#include "application.h"

// Enables WiFiTester 
STARTUP(System.enable(SYSTEM_FLAG_WIFITESTER_OVER_SERIAL1));
STARTUP(System.enableFeature(FEATURE_WIFITESTER));

void setup() {
}

void loop() {
}

References

  • CH7262

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@m-mcgowan m-mcgowan self-requested a review September 7, 2017 17:36
#define SETUP_WIFITESTER_IMPL()
#endif

#define SETUP_WIFITESTER() STARTUP(SETUP_WIFITESTER_IMPL())
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look to me like this macro is used? Is it possible to replace the SETUP_WIFITESTER_IMPL() macro with a regular function? It would be simpler to read and maintain as a function.

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @avtolstoy - any thoughts on that?

Copy link
Member Author

Choose a reason for hiding this comment

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

SETUP_WIFITESTER() is not used and instead we now enable it via STARTUP(System.enableFeature(FEATURE_WIFITESTER));, so it can be removed.

I'll try moving the implementation into a function today.

@m-mcgowan m-mcgowan added this to the 0.8.0 milestone Nov 14, 2017
@avtolstoy avtolstoy force-pushed the feature/ch7262-wifitester-user-module branch from e13f3e4 to 393a77b Compare November 16, 2017 11:04
@m-mcgowan m-mcgowan merged commit 509dff3 into develop Nov 21, 2017
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.

2 participants