Skip to content
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

Accept all line ending styles for USB load #306

Merged
merged 7 commits into from
Apr 10, 2023

Conversation

scanner-darkly
Copy link
Member

What does this PR do?

previously, only *nix style line endings (LF) were accepted. i've updated deserialization code to accept DOS (CRLF) and Mac (CR) line ending styles as well. i also used it as an opportunity to make the deserialization code more robust.

as a consequence of this update, now it will also ignore any lines that start with # and are not followed by M, I, P, G or 1-8, which means script files can be annotated with comments. it also effectively fixes #104.

while working on this, i realized that we use magic numbers for script numbers in multiple places, which makes it more difficult to create firmware mods for increased script count, so i've addressed that as well (i'm also considering revisiting the idea of shadow scripts..)

finally, as there was no need for a dedicated enum for script number, i've changed it to uint8_t and this surfaced an issue in our tests where the script number wasn't set properly, and a couple of places in the main code where we weren't doing boundary checks, so i fixed that as well.

How should this be manually tested?

save/load scenes to USB using different line endings.

If the related Github issues aren't referenced in your commits, please link to them here.

#104

I have,

  • updated CHANGELOG.md and whats_new.md
  • [-] updated the documentation
  • [-] updated help_mode.c (if applicable)
  • run make format on each commit
  • run tests

Copy link
Member

@tehn tehn left a comment

Choose a reason for hiding this comment

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

this is an excellent cleanup, thank you so much for this level of attention. the code feels much better.

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.

scenes with # in description don't load properly
2 participants