Skip to content

Conversation

@Shoeyoung4
Copy link

We created a new implementation of checking continuity to the voltage struct and deleted the continuity struct.

Copy link
Contributor

@mpkarpov-ui mpkarpov-ui left a 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,

Comment on lines +83 to +84
float A_value = sensorA.value* (100000.0/118000); //accounting for the resisters
float A_voltage = (((float)A_value / 65535) * 3.3)/(18.0/100);
Copy link
Contributor

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...

Copy link
Contributor

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"
Copy link
Contributor

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.

Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants