Skip to content

Fix: bugs in simulator config, discovery, and broadband source#172

Open
louislva wants to merge 3 commits intosciencecorp:mainfrom
louislva:simulator-bugfixes
Open

Fix: bugs in simulator config, discovery, and broadband source#172
louislva wants to merge 3 commits intosciencecorp:mainfrom
louislva:simulator-bugfixes

Conversation

@louislva
Copy link

Hey! I was tinkering with the Synapse CLI and found/fixed a few bugs along the way. Here they are, in the same order as the diff shows the files changed:

  1. Config class stored nodes and connections on the class rather than the instance/object. This meant every instance of Config() would share same values for those two properties, which causes problems if you have multiple Config()s
  2. The simulator would let you configure a name with a space inside it, which would cause the client to be unable to discover the device. This is because spaces are used to separate values inside of synapse-python/synapse/utils/discover.py, and so shouldn't appear in any given value.
  3. The broadband source dropped lots of frames if you ran it below 100hz, because of how n_samples was calculated

Copy link
Contributor

@calvinleng-science calvinleng-science left a comment

Choose a reason for hiding this comment

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

Welcome, great first contribution! All the fixes are solid; tested locally and everything checks out.

Let us know if you run into anything else, if there's anything in the direction of usability that you would like us to improve on, or features you would like to see.

You should be prompted for the signing of our CLA in the CLA Check workflow. Once that is completed, you will be able to merge.

And again, welcome!

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.

2 participants