Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

User can specific simulate sample's message sending period #61

Closed
wants to merge 1 commit into from
Closed

User can specific simulate sample's message sending period #61

wants to merge 1 commit into from

Conversation

yuwzho
Copy link
Contributor

@yuwzho yuwzho commented Nov 30, 2016

The simulate sample always send a message every 10 seconds in currect time.

It is different with the BLE sample, which will make the "simlate" sample not simulate the BLE sample's behavior.

@yuwzho
Copy link
Contributor Author

yuwzho commented Dec 1, 2016

@andrew-buckley

Copy link
Contributor

@darobs darobs left a comment

Choose a reason for hiding this comment

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

Look very good overall, once these comments are addressed, I will gladly approve.

Thank you!

@@ -190,7 +199,7 @@ static MODULE_HANDLE SimulatedDevice_Create(BROKER_HANDLE broker, const void* co

static void * SimulatedDevice_ParseConfigurationFromJson(const char* configuration)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be corresponding changes to FreeConfigurationFromJson?

config.messagePeriod = period;
result = malloc(sizeof(SIMULATEDDEVICE_CONFIG));
if (result == NULL) {
LogError("allocation of configuration failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

Free config.macAddress as well, please.

@darobs
Copy link
Contributor

darobs commented Dec 2, 2016

I'm sorry, @yuwzho, I noticed too late that this was a branch off of master, would you please make a pull request from develop?

@yuwzho
Copy link
Contributor Author

yuwzho commented Dec 2, 2016

@darobs Sure, I will fix your comments and send a new pullrequest to the develop branch

@yuwzho
Copy link
Contributor Author

yuwzho commented Dec 2, 2016

Open another one to develop branch and the new one already refers to this one. So close it.

@yuwzho yuwzho closed this Dec 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants