- /* 58 is max packet length for ACKed commands (read prod info) */
- #define FPM_BUFFER_SZ (58)
+ /* 46 is the max packet length for ACKed commands, +1 for confirmation code */
+ #define FPM_BUFFER_SZ (46 + 1)
...because 46 is the maximum packet length for the ReadProdInfo command and the buffer is only ever supposed to hold the packet's payload + 1 byte for the confirmation code, not the entire packet including the header.
...right now, it accepts a pointer and just copies the serial number into that. It relies on the user knowing just how many bytes to allocate for the serial number, which we can avoid here. Considering the product info is made up of proper ASCII strings or integers, I think using a struct to hold the data would allow users easily access all its individual members, not only the serial number, without having to know about the internal workings or packet format, like prod_info.serialNumber or prod_info.templateSize. Something like this:
This way, one could call FPM::getProductInfo(FPM_ProductInfo * prod_info); and access the members easily, especially the strings as already null-terminated. You'd need to copy each part of the ProdInfo packet carefully into the right member of the struct, making sure to swap bytes where necessary since the numbers are in big-endian format.
Thanks. Some changes you should make first:
...because 46 is the maximum packet length for the ReadProdInfo command and the buffer is only ever supposed to hold the packet's payload + 1 byte for the confirmation code, not the entire packet including the header.
For the function itself:
int16_t FPM::getSerialNumber(uint8_t * serialNumber)
...right now, it accepts a pointer and just copies the serial number into that. It relies on the user knowing just how many bytes to allocate for the serial number, which we can avoid here. Considering the product info is made up of proper ASCII strings or integers, I think using a struct to hold the data would allow users easily access all its individual members, not only the serial number, without having to know about the internal workings or packet format, like
prod_info.serialNumber
orprod_info.templateSize
. Something like this:This way, one could call
FPM::getProductInfo(FPM_ProductInfo * prod_info);
and access the members easily, especially the strings as already null-terminated. You'd need to copy each part of the ProdInfo packet carefully into the right member of the struct, making sure to swap bytes where necessary since the numbers are in big-endian format.