[example] Linux Example App - Light #2365
Conversation
erjiaqing
requested review from
andy31415,
BroderickCarlin,
chrisdecenzo,
hawk248,
jelderton,
robszewczyk,
rwalker-apple and
saurabhst
as code owners
erjiaqing
changed the title
erjiaqing
changed the title
|
|
||
| if (waitingForResponse) | ||
| std::unique_lock<std::mutex> lk(cvWaitingForResponseMutex); |
There was a problem hiding this comment.
@bzbarsky-apple will you have a look here?
There was a problem hiding this comment.
This part looks fine to me, given that we set waitingForResponse to true before we call SendMessage, so even a sync callback under there will do the right thing.
|
|
||
| include_dirs = [ | ||
| ".", | ||
| "${chip_root}/src/app/util", |
There was a problem hiding this comment.
would prefer that we used #include <app/util/XXX.h> in source files and and have -I${chip_root}/src if possible. if there's an issue with a file in gen/ that makes this pattern untenable, lemme know
There was a problem hiding this comment.
There are a bunch of includes in gen files of af.h, af-types.h, util.h. More importantly, there are includes like that in the non-gen files too. We could fix that by adding ${chip_root}/src to the include path for all the apps involved, then changing the non-gen files, then changing the various gen files in the apps, then removing ${chip_root}/src/app/util from the include paths...
I would argue for not trying to do that as part of this PR and doing a followup instead.
| @@ -53,33 +55,37 @@ | |||
| using namespace ::chip; | |||
| using namespace ::chip::Inet; | |||
|
|
|||
| namespace { | |||
@erjiaqing can you please phrase a problem for this PR? |
erjiaqing
force-pushed
the
linux-chip-device
branch
from
8459f86
to
fe5dd08
Compare
|
@erjiaqing need conflicts addressed |
erjiaqing
force-pushed
the
linux-chip-device
branch
from
aa58ce1
to
6b788fd
Compare
erjiaqing
force-pushed
the
linux-chip-device
branch
2 times, most recently
from
f70a26f
to
1d42bbe
Compare
[examples] Initial lighting app on linux platform nit address comments move cv outside the lock scope Followup project-chip#2413 Fix build
erjiaqing
force-pushed
the
linux-chip-device
branch
from
1d42bbe
to
a0911fa
Compare
Problem
Currently, there is a chip-server in chip-tool acts as a lighting app for testing use comes with the initial PR with cirque tests, which is a bit hack, and unclean. Also we need a Lighting app for linux as a example, it is clear that merge the two app into one will be better.
Summary of Changes
Note
issue #2302
fix #1761