-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[cirque] Initial support for Cirque test #2042
Conversation
examples/chip-tool/server.cpp
Outdated
|
||
constexpr chip::NodeId kLocalNodeId = 12344321; | ||
|
||
#define println(format, ...) printf(format "\n", ##__VA_ARGS__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we just use printf and avoid the extra indirection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you re-upload the branch to update? I don't seem to see updated changes.
examples/chip-tool/server.cpp
Outdated
class ServerCallback : public SecureSessionMgrCallback | ||
{ | ||
public: | ||
virtual void OnMessageReceived(const MessageHeader & header, Transport::PeerConnectionState * state, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest using 'override' instead of virtual to make it clear we override a base method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
examples/chip-tool/server.cpp
Outdated
} | ||
}; | ||
|
||
static ServerCallback gCallbacks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static not needed in anonymous namespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
fp.write(ret_log.log) | ||
|
||
|
||
def create_device_from_config(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we use DEVICE_CONFIG.values() insead?
I can only see the config used as a list - should it be a list instead of a dict?
|
||
device_ids = set() | ||
|
||
for device in self.device_config: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use for device in self.device_config.values()
and use **device
instead of **self.device_config[device]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer applied since we switched to flask api
except: | ||
traceback.print_exc(file=sys.stdout) | ||
test_success = False | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pass here and below do not seem needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by moving test warpper to base class
continue | ||
self.logger.info("return ip: {}".format(ipstr)) | ||
return str(ipaddr) | ||
except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exceptions can happen here?
Could you add a comment about them and why it is appropriate to silently ignore them? I am used to at least logging exceptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment to explain why just ignore the exception
b9d5f38
to
04117e1
Compare
return emberAfSendInterPan(panId, destinationLongId, destinationShortId, multicastId, emAfCommandApsFrame->clusterId, profileId, | ||
*emAfResponseLengthPtr, emAfZclBuffer); | ||
} | ||
// EmberStatus emberAfSendCommandUnicastToBindingsWithCallback(EmberAfMessageSentFunction callback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions will not be called and will block chip-server from linking, so comment them out.
|
||
return true; | ||
} | ||
// bool emberAfEndpointEnableDisable(uint8_t endpoint, bool enable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions will not be called and will block chip-server from linking, so comment them out.
emberAfGetEui64(myEui64); | ||
return (0 == memcmp(eui64, myEui64, EUI64_SIZE) ? true : false); | ||
} | ||
// EmberStatus emberAfEndpointEventControlSetDelayMS(EmberEventControl * controls, uint8_t endpoint, uint32_t delayMs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions will not be called and will block chip-server from linking, so comment them out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do they block chip-server from linking?
04117e1
to
bc3d870
Compare
I guess #1761 can be implemented first so we can have a valid chip server implementation. |
@@ -0,0 +1,87 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need these files under examples/chip-tool/gen
? I personally believe they should be in the core library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to current status, seems we will still put them in seperate example directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this particular file is generated specifically for the app, is different for every collection of clusters
863ee7c
to
c4bef25
Compare
- Adds a zcl server for posix mock devices. - Adds the dockerfile for integration into cirque.
c4bef25
to
bfd0a7a
Compare
Size increase report for "nrf-example-build"
Full report output
|
Size increase report for "nrfconnect-example-build"
Full report output
|
Size increase report for "linux-example-build"
Full report output
|
Size increase report for "esp32-example-build"
Full report output
|
Size increase report for "gn_nrf-example-build"
Full report output
|
Size increase report for "gn_linux-example-build"
Full report output
|
@erjiaqing Can we try to get this integrated into GitHub workflows? |
I will submit another PR to integrate this into Github workflows. |
Summary of Changes
How to test
do_on-off-cluster-test.sh
Note:
fixes #1891