Add new adapter layer#2
Open
jfaeh wants to merge 2 commits into
Open
Conversation
Renamade from old sensirion_common to sensirion_conversions
Rol-la
approved these changes
Jun 4, 2026
Rol-la
reviewed
Jun 4, 2026
|
|
||
| // copy the bytes from source to destination. The source is in big-endian/MSB-first format, so the first byte of the source is the most significant byte. | ||
| // The destination should be filled in the correct order for the system endianness. | ||
| /* copy the bytes from source to destination. The source is in big-endian/MSB-first format, |
Collaborator
There was a problem hiding this comment.
I think we have to consider the byte order of the host. In case the host has the same byte order (BE) this code is wrong.
I'm aware that his is also the case for the code we have in sensirion_common :-(
Rol-la
reviewed
Jun 4, 2026
| if (packet->crc8_poly != 0) { | ||
| packet->data[offset] = sensirion_i2c_packet_get_crc(packet, offset - SENSIRION_WORD_SIZE); | ||
| for (i = 0; i < data_length; i += 2u) { | ||
| i2c_packet->data[offset++] = data[i]; |
Collaborator
There was a problem hiding this comment.
how about of doing a memcpy?
Rol-la
reviewed
Jun 4, 2026
| uint16_t num_words = (data_length + has_8bit_command) / SENSIRION_WORD_SIZE; | ||
| uint8_t tx_buffer[SENSIRION_WORD_SIZE * num_words + SENSIRION_CRC8_LEN * num_words]; | ||
|
|
||
| /*skips the first word (command) because the command does not have a CRC*/ |
Collaborator
There was a problem hiding this comment.
if we consistently use the i2c_packet_add_ there is no need to add a crc again. I also think the i2c_packet* should be the first arguement and the i2c_spec the last one!
Rol-la
reviewed
Jun 4, 2026
| } | ||
| packet->data[j++] = packet->data[i]; | ||
| packet->data[j++] = packet->data[i + 1]; | ||
| i2c_packet->data[j++] = i2c_packet->data[i]; |
Collaborator
There was a problem hiding this comment.
also here we may use a memcpy
Rol-la
reviewed
Jun 4, 2026
| * @return uint8_t Calculated CRC8 value. | ||
| */ | ||
| uint8_t sensirion_i2c_packet_get_crc(const i2c_packet *packet, uint16_t index); | ||
| uint8_t sensirion_i2c_packet_get_crc(const i2c_packet_t *i2c_packet, uint16_t index); |
Collaborator
There was a problem hiding this comment.
in fact we may use a const i2c_packet_t* const
Rol-la
reviewed
Jun 4, 2026
| * @return false If the CRC8 is invalid. | ||
| */ | ||
| bool sensirion_i2c_packet_check_crc(const i2c_packet *packet, uint16_t index); | ||
| bool sensirion_i2c_packet_check_crc(const i2c_packet_t *i2c_packet, uint16_t index); |
Collaborator
There was a problem hiding this comment.
in fact we may use a const i2c_packet_t* const
Rol-la
reviewed
Jun 4, 2026
| */ | ||
| int sensirion_i2c_packet_write(const i2c_packet const *packet, uint16_t data_length, | ||
| const struct i2c_dt_spec *i2c_spec); | ||
| int sensirion_i2c_packet_write(const struct i2c_dt_spec *i2c_spec, const i2c_packet_t *i2c_packet, |
Collaborator
There was a problem hiding this comment.
why do you think it is better to have the first argument the i2c_spec?
Rol-la
reviewed
Jun 4, 2026
| */ | ||
| int sensirion_i2c_packet_read(const i2c_packet *i2c_packet, uint16_t expected_data_length, | ||
| const struct i2c_dt_spec *i2c_spec); | ||
| int sensirion_i2c_packet_read(const struct i2c_dt_spec *i2c_spec, const i2c_packet_t *i2c_packet, |
Collaborator
There was a problem hiding this comment.
why do you think it is better to have the first argument the i2c_spec?
Rol-la
reviewed
Jun 4, 2026
| * @return The byte array represented as float | ||
| */ | ||
| float sensirion_common_bytes_to_float(const uint8_t *bytes); | ||
| float sensirion_conversions_bytes_to_float(const uint8_t *bytes); |
Collaborator
There was a problem hiding this comment.
could also be const uint8_t const*
Rol-la
requested changes
Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This Pull Request adds an adapter layer and updates the sts4x sensor driver for the new adapter layer.