const Parameters? - Githubissues

RobTillaart / CRC

CRC library for Arduino

MIT License

79 stars 17 forks source link

const Parameters? #11

Closed ModischFabrications closed 2 years ago

ModischFabrications commented 2 years ago

crc* functions don't modify the array passed, shouldn't they be marked as const? This would allow these function to handle string-type input a lot better.

uint16_t crc16(const uint8_t* array, ....

Test: crc16("Test123", 16);

RobTillaart commented 2 years ago

Thanks for pointing out, I did not test that yet. Did you ran a test to see the gain? even a few micros will add up!

ModischFabrications commented 2 years ago

Haven't benchmarked it yet, but even the convenience and safety of passing const is worth it in my opinion.

RobTillaart commented 2 years ago

Quick test on AVR with the performance sketch shows no difference. Compiler seems smart enough.

But I noticed that the length parameter was not implemented consequently uint8_t vs uint32_t. So that will all be updated to uint8_t as that should be large enough for most (Arduino) applications.

RobTillaart commented 2 years ago

Created branch - https://github.com/RobTillaart/CRC/tree/Fix11

changed parameters to const
changed length to uint8_t

prepared as a new version 0.1.5

RobTillaart commented 2 years ago

PR created - https://github.com/RobTillaart/CRC/pull/12

can you verify that Fix11 branch works for you? if so I will merge it into master branch.

ModischFabrications commented 2 years ago

Added comments to the PR, i think const for value-types are not needed though. Looks good nonetheless, feel free to close this issue.