Skip to content

Conversation

@8ohamed
Copy link
Collaborator

@8ohamed 8ohamed commented Mar 24, 2025

No description provided.

Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

@8ohamed thanks for the new PR. Please see the comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an advantage of having different cases for configuration in one json file?

Copy link
Contributor

Choose a reason for hiding this comment

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

please apply the suggestions on this file from PR #7

sorted_keys = sorted(self.data_map.keys())

collected_samples = []
samples_collected = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be obtained from the size of the previous list?

@8ohamed
Copy link
Collaborator Author

8ohamed commented Mar 28, 2025

@prasadtalasila
I have pushed the changes, but for the mqtt.json having different cases for configuration in one json file makes it easier accessing them, and knowing which server to use, in case of using the cp-sens simulator or the public server for testing.

@prasadtalasila
Copy link
Contributor

@prasadtalasila
I have pushed the changes, but for the mqtt.json having different cases for configuration in one json file makes it easier accessing them, and knowing which server to use, in case of using the cp-sens simulator or the public server for testing.

It is best to have separate config files for that purpose. For example

test.json
production.json
r-pi.json

Another suggestion. The files themselves will have credentials. May be it is best to have

test.json.template
production.json.template
r-pi.json.template

and add config/*.json to .gitignore file. The user/developer can copy templates into json files, and use them locally.

@prasadtalasila
Copy link
Contributor

@8ohamed please let me know when the changes are complete. Thanks.

@8ohamed
Copy link
Collaborator Author

8ohamed commented Mar 29, 2025

@prasadtalasila Thanks for the suggestions. I have now pushed the changes.

@8ohamed
Copy link
Collaborator Author

8ohamed commented Apr 1, 2025

@prasadtalasila thanks for your help earlier today.
I have pushed the last fixes where I fixed the pylint issues we talked about.
While running experiment I faced problems where I was not getting any message, then I tried different topics and it worked, so the code and the logic works. If you face problems maybe try different topics than those in the template.

@prasadtalasila prasadtalasila merged commit 43b3560 into main Apr 1, 2025
@prasadtalasila prasadtalasila deleted the data-processing branch April 7, 2025 09:36
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.

4 participants