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

Improve wificfg #732

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Improve wificfg #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58b9932
extras/wificfg: make sure wificfg.h #includes all necessary files
zub2 Oct 3, 2019
5f6ccbd
extras/wificfg: use #if when checking for configUSE_TRACE_FACILITY
zub2 Oct 3, 2019
77b2b23
extras/wificfg: make the default wifi options overridable
zub2 Oct 3, 2019
a4defa3
extras/wificfg: fix a typo in comment
zub2 Oct 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions extras/wificfg/wificfg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@
#endif


const char *wificfg_default_ssid = "EOR_%02X%02X%02X";
const char *wificfg_default_password = "esp-open-rtos";
const char *wificfg_default_hostname = "eor-%02x%02x%02x";
const char *wificfg_default_ssid __attribute__ ((weak)) = "EOR_%02X%02X%02X";
const char *wificfg_default_password __attribute__ ((weak)) = "esp-open-rtos";
const char *wificfg_default_hostname __attribute__ ((weak)) = "eor-%02x%02x%02x";
Copy link
Contributor

Choose a reason for hiding this comment

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

These don't need to be 'weak', they are variables that can be trivially modified, just set them in your app initialization code.

Copy link
Author

Choose a reason for hiding this comment

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

You are right. I didn't realize it's a const char*, not a const char[].

I was motivated by several things:

  • I thought I can't really change it (which was wrong)
  • I didn't want the literals from the wificfg.c to be present in the binary at all (just a few bytes, but still needless if I'm using my own strings)
  • when re-defining a weak symbol, there is no need to do anything at runtime, linker just handles that (yes, just 3 assignments, but when just a declaration can suffice, it seems nicer)

Because the type is const char*, even the second point was not fulfilled. If the type is changed to const char[], then it seems to do what I wanted.

What do you think about keeping these weak and making them const char[]? Or are the last two points from the list above not interesting to you?


/* The http task stack allocates a single buffer to do much of it's work. */
/* The http task stack allocates a single buffer to do much of its work. */
#define HTTP_BUFFER_SIZE 54

/*
Expand Down Expand Up @@ -1540,7 +1540,7 @@ static int handle_wificfg_challenge_post(int s, wificfg_method method,
return wificfg_write_string(s, http_redirect_header);
}

#ifdef configUSE_TRACE_FACILITY
#if configUSE_TRACE_FACILITY
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that looks right, thanks.

static const char *http_tasks_content[] = {
#include "content/tasks.html"
};
Expand Down Expand Up @@ -1638,7 +1638,7 @@ static const wificfg_dispatch wificfg_dispatch_list[] = {
{"/challenge.html", HTTP_METHOD_POST, handle_wificfg_challenge_post, false},
{"/wificfg/restart.html", HTTP_METHOD_POST, handle_restart_post, true},
{"/wificfg/erase.html", HTTP_METHOD_POST, handle_erase_post, true},
#ifdef configUSE_TRACE_FACILITY
#if configUSE_TRACE_FACILITY
{"/tasks", HTTP_METHOD_GET, handle_tasks, false},
{"/tasks.html", HTTP_METHOD_GET, handle_tasks, false},
#endif /* configUSE_TRACE_FACILITY */
Expand Down
4 changes: 4 additions & 0 deletions extras/wificfg/wificfg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
extern "C" {
#endif

#include <stddef.h>
#include <stdint.h>
#include <sys/types.h>

/*
* Printf format used to initialize a default AP ssid. It is passed the last
* three bytes of the mac address. This may be NULL to not default the ssid,
Expand Down