-
Notifications
You must be signed in to change notification settings - Fork 0
New ADC implementation #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
mpkarpov-ui
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is mostly good, only requesting changes for some of the magic number stuff in my comments. Will approve after that :)
Overall great work though, looks functionally good & I witnessed it working as well,
| float A_value = sensorA.value* (100000.0/118000); //accounting for the resisters | ||
| float A_voltage = (((float)A_value / 65535) * 3.3)/(18.0/100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love maybe to move some of these numbers to defines or something, it does look like some "magic math" is happening here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^
| @@ -1,4 +1,4 @@ | |||
| #include "silsim/emulation.h" | |||
| //#include "silsim/emulation.h" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very curious change, but i care more about it compiling on FSW than working on SILSIM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably do hilsim updates post luna or whatever, so dont worry abt silsim stuff.
We created a new implementation of checking continuity to the voltage struct and deleted the continuity struct.