Skip to content

Fix Sporadic Temperature Reporting - #41

Merged
CoolSpy3 merged 18 commits into
masterfrom
no-temp-limit
Jul 9, 2023
Merged

Fix Sporadic Temperature Reporting#41
CoolSpy3 merged 18 commits into
masterfrom
no-temp-limit

Conversation

@CoolSpy3

Copy link
Copy Markdown
Member

No description provided.

@CoolSpy3
CoolSpy3 requested a review from brettleApril 2, 2023 05:57
@CoolSpy3CoolSpy3 changed the title Fix Sparatic Temperature ReportingFix Sporadic Temperature ReportingApr 2, 2023
Comment threadsrc/main/java/org/carlmontrobotics/lib199/MotorErrors.java
@CoolSpy3
CoolSpy3 requested a review from brettleApril 2, 2023 06:24

@brettlebrettle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but recommend waiting on this until after comp. Note that your fix only addresses sporadic temp reporting. If the temp sensor is broken in a way that causes it to consistently report a false high temp, the motor will still be shut down. Since the 2023 bot still has a motor with a questionable temp sensor on it, this remains a risky change.

@CoolSpy3

Copy link
Copy Markdown
MemberAuthor

Looks good but recommend waiting on this until after comp. Note that your fix only addresses sporadic temp reporting. If the temp sensor is broken in a way that causes it to consistently report a false high temp, the motor will still be shut down. Since the 2023 bot still has a motor with a questionable temp sensor on it, this remains a risky change.

I'm fine to wait until after comp. In my opinion, a motor with a faulty temp sensor should be disabled because 1) we cannot safely ensure it stays in a safe temperature range, and 2) If the temp sensor is broken, it's not improbable that there is damage to the motor or encoder as well.

Comment threadsrc/main/java/org/carlmontrobotics/lib199/MotorErrors.java
Comment threadsrc/main/java/org/carlmontrobotics/lib199/MotorErrors.java
Comment threadsrc/test/java/org/carlmontrobotics/lib199/MotorErrorsTest.java Outdated
Comment threadsrc/main/java/org/carlmontrobotics/lib199/MotorErrors.java
Comment threadsrc/main/java/org/carlmontrobotics/lib199/MotorConfig.java
Comment threadsrc/test/java/org/carlmontrobotics/lib199/MotorErrorsTest.java Outdated

@ProfessorAtomicManiacProfessorAtomicManiac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me! Just gotta test it. I'm assuming you can just run Test Robot Code in lib199, but you intend to do other tests?

Comment threadsrc/main/java/org/carlmontrobotics/lib199/MotorErrors.java
@CoolSpy3

Copy link
Copy Markdown
MemberAuthor

Everything looks good to me! Just gotta test it. I'm assuming you can just run Test Robot Code in lib199, but you intend to do other tests?

We probably should test on the actual robot to see if it fixes the arm problem.

@CoolSpy3

Copy link
Copy Markdown
MemberAuthor

Having something that records these kinds of errors in an organized manner instead of spiting them to Driver Logs might be a good feature for your GUI :)

Careful. You'll give me ideas ;P

@CoolSpy3CoolSpy3 added the Passed Testing This PR was successfully tested on a robot label Jun 28, 2023
@CoolSpy3CoolSpy3 added the Breaking Change This PR will require an increment to the lib199 major version number label Jun 28, 2023
@CoolSpy3CoolSpy3 added the bug Something isn't working label Jun 30, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking ChangeThis PR will require an increment to the lib199 major version numberbugSomething isn't workingPassed TestingThis PR was successfully tested on a robot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Temperature checking is occasionally reporting bogus temperatures and disabling motors.

3 participants

@CoolSpy3@brettle@ProfessorAtomicManiac