Skip to content

Conversation

@sfe-SparkFro
Copy link
Collaborator

Will resolve #9

I have not yet changed any names, just ran the files through a formatter. @gigapod, you mentioned wanting to change some names away from snake case. I'm also not sold on all the names I chose. Do you have any suggestions?

Clang formatter in VS Code set to Microsoft style
Clang formatter in VS Code set to Microsoft style
@sfe-SparkFro
Copy link
Collaborator Author

Also, are the file names and directory structure okay? If we're going to change those, would probably be best to do that as part of this PR. And any other house-cleaning things like that, really.

Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

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

Overall looks good. The only thing to tweak is the class names - see comment I left.

@PaulZC
Copy link
Contributor

PaulZC commented Dec 15, 2023

u-blox is preferred.
ublox 2nd choice.
(They prefer without capitalisation....)

Many command and response arrays are fixed length, so no need to use calloc. This helps prevent the heap from getting fragmented.
clang with Microsoft style
@sfe-SparkFro
Copy link
Collaborator Author

@gigapod @PaulZC Changes pushed. Also decided to address #12 in this PR, and I removed a bunch of calls to ubx_cell_calloc_char() in favor of putting arrays on the stack; should help with preventing the heap from getting fragmented, and give a minor performance improvement.

Please take a look and let me know what you think! If all is okay, I'd like to get the first release out this week if possible. There's still a couple open issues, but it should at least be totally useable.

@sfe-SparkFro
Copy link
Collaborator Author

@PaulZC @gigapod Last call for input before I merge this early next week. Thanks!

Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

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

Looks good - just one very minor comment/nit.

@sfe-SparkFro sfe-SparkFro merged commit 13ffd73 into v1.0.0 Jan 4, 2024
@sfe-SparkFro sfe-SparkFro deleted the format_library branch January 4, 2024 19:53
@sfe-SparkFro sfe-SparkFro mentioned this pull request Jan 9, 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.

4 participants