Skip to content

Fix custom instruments getting empty name and ID #9

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

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

Bentroen
Copy link
Member

@Bentroen Bentroen commented Feb 5, 2024

Prerequisites

  • A similar pull request does not already exist, or this pull request supersedes an existing pull request.
  • The project's contributing and relevant documentation have been acknowledged and adhered to.
  • I have signed off all of my relevant commits.

Closes #8

Information

As per issue #8, during the parsing of an NBS file, custom instruments are generated that contain empty meta.name (empty string) and id (NaN) attributes. This results from the name string being read from the file, but not actually stored in the name attribute, which receives the default value "" from the defaultInstrumentOptions object.

Additionally, as the first parameter to the Instrument object constructor is passed the return value of Number.parseInt called on the parsed string, which always results in NaN. As a result, this value is assigned to all custom instruments' id attributes.

This pull request fixes the issue by calculating the custom instrument's ID from the firstCustomIndex property and the index of the current instrument being read, and assigning the parsed name string to the meta.name property.

@encode42
Copy link
Member

encode42 commented Feb 6, 2024

Thanks for the research! This happened during a minor refactor for v4. I don't know how this was missed. Clearly I need to write more tests, but that'll hopefully be dealt with soon when I start using the project more myself.

@encode42 encode42 merged commit f88edfc into OpenNBS:main Feb 6, 2024
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.

Custom instruments have empty name
2 participants