updated sht driver + header #172
Conversation
general/src/sht30.c
Outdated
| } | ||
|
|
||
| temp = uint8_to_uint16(data.databuf[0], data.databuf[1]); | ||
| sht30->delay(1u); |
There was a problem hiding this comment.
fair, but lets simplify and not put a delay here, it shouldnt actually be needed. alllows us to remove the delay func ptr entirely
general/src/sht30.c
Outdated
|
|
||
| sht30->temp = (uint16_t)val; | ||
| uint32_t temp_val = (uint32_t)(175000u * (uint32_t)temp / 65535u - 45000u); |
There was a problem hiding this comment.
these calcs were changed; thats not in scope of this ticket and i dont feel like checking chatgpts math so can we revert
There was a problem hiding this comment.
I thought we weren't supposed to use floats directly as per the style guide?
There was a problem hiding this comment.
Fair, I guess I should be more clear in that bc that's a good catch. That part of the style guide refers to data storage. In this case, it's just use as an intermediary value- the final value that's being stored is an int.
It's okay that values along the way are floats.
Either way, we def try to keep tickets to the scope that they are listed as. If you spot anything u think should be addressed otherwise, you can make another ticket to address that
There was a problem hiding this comment.
sound good, i'll update the PR after I fix everything up
general/include/sht30.h
Outdated
| I2C_HandleTypeDef *i2c_handle; | ||
| uint16_t status_reg; | ||
| I2C_TransmitFunc i2c_transmit; | ||
| I2C_ReceiveFunc i2c_receive; |
There was a problem hiding this comment.
nitpick but can we change the naming here :
I2C_TransmitFunc --> Write_ptr
I2C_ReceieveFunc --> Read_Ptr
I2C_MemReadFunc --> Mem_Read_Ptr
then the instances to "read', "write"m "mem_read"
|
changes made |
There was a problem hiding this comment.
there's still a ton of changes here that are not in scope of this ticket. The temp calculation is still done differently, and every single number still ends in "u" - even in areas that you obviously didnt otherwise edit. We dont use that convention; very few people do - auto code generators are the only places who will spit that out regularly. Please revert these changes
| static int sht30_write_reg(sht30_t *sht30, uint16_t command) | ||
| { | ||
| uint8_t command_buffer[2] = { (command & 0xff00u) >> 8u, | ||
| command & 0xffu }; |
There was a problem hiding this comment.
this still uses u after all ints. please remove
There was a problem hiding this comment.
I'm confused, the verison in main uses u? I didn't touch it since it wasn't in the scope of the ticket, but i'll remove it and recommit without the .ds files
Changes
generalized sht driver and header
Notes
To Do
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please reach out to your Project Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
Closes # (issue #160 )