Better lights for robot #98

The-Arx · 2024-03-21T03:51:27Z

Description

Allows both halves of lights to be turned on independently. Currently second half has none but we can add stuff to signal to the player.

How Has This Been Tested?

Untested because robot was unavailable

dejabot · 2024-03-21T08:02:37Z

src/main/java/com/team766/robot/reva/DriverOI.java

@@ -108,6 +108,8 @@ public void handleOI(Context context) {
            drive.setCross();

            context.releaseOwnership(drive);
+


comment - what does turning off the front signify?

also, don't forget to take and release ownership of lights

src/main/java/com/team766/robot/reva/mechanisms/ForwardApriltagCamera.java

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

dejabot · 2024-03-21T08:07:02Z

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

+    private static final int BACK_OFFSET = 48;
+    private static final int BACK_SLOT = 2;
+
+    public void signalPlayersGreen() {


each of these signal* or turnOffFront methods should check context ownership

calling Procedures should take and release ownership as appropriate.

I can help you with this when we talk.

Agree, +1. @dejabot can you please help make sure this gets into the codebase so it is ready before tomorrow morning?

The check happens inside of the setAreaColor and animate methods.

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

dejabot · 2024-03-21T08:09:47Z

src/main/java/com/team766/robot/reva/procedures/IntakeUntilIn.java

@@ -19,6 +19,6 @@ public void run(Context context) {

        context.waitForSeconds(2);

-        Robot.lights.turnLightsOff();
+        Robot.lights.turnOffFront();


do you want to signal that there's now a note in the intake?
btw what does turn off front signify?

That would be a good idea. I think the sample code I pushed to main didn't include this on accident. My bad.

It did have that but was just collapsed by GitHub when reviewing I think

src/main/java/com/team766/robot/reva/procedures/IntakeUntilIn.java

dejabot · 2024-03-21T08:10:20Z

src/main/java/com/team766/robot/reva/DriverOI.java

@@ -169,6 +171,10 @@ public void handleOI(Context context) {
            context.takeOwnership(drive);
            drive.stopDrive();
        }
+
+        if (isRotatingToSpeaker && drive.isAtRotationTarget() && Robot.shoulder.isFinished()) {
+            Robot.lights.signalCanShoot();


take and release ownership of lights

Agree with debajit, + 1
Also, this code seems like it would make the lights flash on/off a lot while Luis is moving the robot, and then will be static once Luis stops the robot. Would it be better to make it so there is one lights sequence if the robot is in lock mode and another in the procedure if the robot has started the actual shooting procedure?

This is what Raj asked me to implement. It turns on the light when it is ready but waits until Louis releases the button to turn it off.

dejabot · 2024-03-21T08:21:11Z

nice job with this! I think it's close. to avoid having multiple procedures accidentally trying to use the lights simultaneously, you should check context ownership in Lights for any methods that modify the state of the lights, and you should take and release ownership for any procedures (or OI fragments) that call those methods.

it might also be helpful to comment about what the front and back are representing.

ultimately, it'd be great to write up markdown docs (in docs/) for what the lights mean.

thank you!

maxspier

This is almost perfect! I think there's just a few things to talk over and discuss with @dejabot, and then implement, and then we can get this merged to main before tomorrow! Great work, especially with the split LED strip design! All of this code should be really useful on the robot!

maxspier · 2024-03-21T22:59:18Z

src/main/java/com/team766/robot/reva/DriverOI.java

@@ -169,6 +171,10 @@ public void handleOI(Context context) {
            context.takeOwnership(drive);
            drive.stopDrive();
        }
+
+        if (isRotatingToSpeaker && drive.isAtRotationTarget() && Robot.shoulder.isFinished()) {
+            Robot.lights.signalCanShoot();


Agree with debajit, + 1
Also, this code seems like it would make the lights flash on/off a lot while Luis is moving the robot, and then will be static once Luis stops the robot. Would it be better to make it so there is one lights sequence if the robot is in lock mode and another in the procedure if the robot has started the actual shooting procedure?

src/main/java/com/team766/robot/reva/mechanisms/ForwardApriltagCamera.java

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

maxspier · 2024-03-21T23:02:32Z

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

+    private static final int BACK_OFFSET = 48;
+    private static final int BACK_SLOT = 2;
+
+    public void signalPlayersGreen() {


Agree, +1. @dejabot can you please help make sure this gets into the codebase so it is ready before tomorrow morning?

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

maxspier · 2024-03-21T23:07:10Z

src/main/java/com/team766/robot/reva/procedures/IntakeUntilIn.java

@@ -19,6 +19,6 @@ public void run(Context context) {

        context.waitForSeconds(2);

-        Robot.lights.turnLightsOff();
+        Robot.lights.turnOffFront();


That would be a good idea. I think the sample code I pushed to main didn't include this on accident. My bad.

maxspier · 2024-03-21T23:11:19Z

src/main/java/com/team766/robot/reva/mechanisms/Lights.java

There's something to be said about the use of context taking ownership using the Lights class. Theoretically, we would not need to take or release ownership of the lights. However, it does allow for a better handoff between mechanisms/oi/procedure. This could be a thing to discuss with @dejabot ; he should be able to help you with this decision, and then if you decide to implement context he (or I) can help you implement it. Some of the comments I made are assuming we are using this context based lights code, so if we are then you should probably keep them, or otherwise just disregard them.

dejabot · 2024-03-22T18:02:31Z

re: context ownership - having the Lights mechanism check for context ownership, and having calling procedures/OIFragments takeOwnership and releaseOwnership of the context is the right thing to do. The Lights mechanism has mutable state, and you want to ensure that two different procedures etc aren't clobbering each other; that's what the context ownership checks protect.

that said, since we're at comp now and want to minimize "wiggling the jello", I think it's fine not to put in the context ownership checks and do the take/release ownership calls in the procedures/oi. we can add that after comp. unlike a moving mechanism, there's less danger of actual physical clobbering with accidental concurrent manipulation, so we should be okay.

go ahead and proceed with the code without context ownership logic. we can add it after the comp.

excited to see this in action!!

…better-lights

The-Arx added 2 commits March 20, 2024 19:53

Added new light class

3402fb8

Integrated lights into intake and shooter

63fb258

The-Arx requested review from maxspier and dejabot March 21, 2024 03:51

The-Arx self-assigned this Mar 21, 2024

dejabot reviewed Mar 21, 2024

View reviewed changes

maxspier requested changes Mar 21, 2024

View reviewed changes

The-Arx and others added 5 commits March 22, 2024 16:23

Added context ownersip and updated a few other things

f79ba2a

Merge branch 'main' into better-lights

6db18bd

fixed code

2ace978

Merge branch 'better-lights' of https://github.com/Team766/2024 into …

f7376d2

…better-lights

removed ownership check from constructor (not sure if it is necessary)

b9152be

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better lights for robot #98

Better lights for robot #98

The-Arx commented Mar 21, 2024

dejabot Mar 21, 2024

dejabot Mar 21, 2024

maxspier Mar 21, 2024

The-Arx Mar 22, 2024

dejabot Mar 21, 2024

maxspier Mar 21, 2024

The-Arx Mar 23, 2024

dejabot Mar 21, 2024

maxspier Mar 21, 2024

The-Arx Mar 22, 2024

dejabot commented Mar 21, 2024

maxspier left a comment

maxspier Mar 21, 2024

maxspier Mar 21, 2024

maxspier Mar 21, 2024

maxspier Mar 21, 2024

dejabot commented Mar 22, 2024

		@@ -108,6 +108,8 @@ public void handleOI(Context context) {
		drive.setCross();

		context.releaseOwnership(drive);

Better lights for robot #98

Are you sure you want to change the base?

Better lights for robot #98

Conversation

The-Arx commented Mar 21, 2024

Description

How Has This Been Tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dejabot commented Mar 21, 2024

maxspier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dejabot commented Mar 22, 2024