Skip to content
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

Remove usage of robotInit from FRC Docs #2689

Closed
wants to merge 1 commit into from
Closed

Conversation

DJB755
Copy link

@DJB755 DJB755 commented Aug 4, 2024

Remove usage of robotInit #2679

@DJB755 DJB755 closed this Aug 4, 2024
@DJB755
Copy link
Author

DJB755 commented Aug 4, 2024

Oop realized I did not go throught the examples linked in the docs, will do that before submitting the PR

@DJB755 DJB755 reopened this Aug 6, 2024
@DJB755
Copy link
Author

DJB755 commented Aug 6, 2024

Remove usage of robotInit #2679 replaced with different constructor on the FRCDocs side, should I fork allwpilib too and change the examples there too before?

Copy link
Collaborator

@sciencewhiz sciencewhiz left a comment

Choose a reason for hiding this comment

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

In general, this needs more nuanced changes. I commented on a few places at the beginning, but these "rules" apply many more places.

  • Can't say Robot method or function, need to update those to say constructor
  • When there's a description of a side effect that the user needs to be aware of, it's still appropriate to reference robotInit, as users can still use robotInit, we just don't want to encourage it's use with examples.
  • There's cases where code samples say void Robot() these need to be updated since Robot isn't a method
  • I suspect that the python changes aren't correct
  • For the stacktraces article, I suspect the stack traces look significantly different when run from the constructor. This can't be updated just with find and replace. Per the previous point, I don't think this article is important to update as showing robotInit is ancillary to the point of explaining stack traces, but it would be bad to update the way it is that makes it wrong.

@@ -67,7 +67,7 @@ Here is an example of generating a trajectory using clamped cubic splines for th

.. note:: The Java code utilizes the `Units <https://github.wpilib.org/allwpilib/docs/release/java/edu/wpi/first/math/util/Units.html>`_ utility, for easy unit conversions.

.. note:: Generating a typical trajectory takes about 10 ms to 25 ms. This isn't long, but it's still highly recommended to generate all trajectories on startup (``robotInit``).
.. note:: Generating a typical trajectory takes about 10 ms to 25 ms. This isn't long, but it's still highly recommended to generate all trajectories on startup (``Robot``).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should say something like Robot constructor

@@ -3,7 +3,7 @@ Get Alliance Color

The ``DriverStation`` class (`Java <https://github.wpilib.org/allwpilib/docs/release/java/edu/wpi/first/wpilibj/DriverStation.html>`__, `C++ <https://github.wpilib.org/allwpilib/docs/release/cpp/classfrc_1_1_driver_station.html>`__, :py:class:`Python <robotpy:wpilib.DriverStation>`) has many useful features for getting data from the Driver Station computer. One of the most important features is ``getAlliance`` (Java & Python) / ``GetAlliance`` (C++).

Note that there are three cases: red, blue, and no color yet. It is important that code handles the third case correctly because the alliance color will not be available until the Driver Station connects. In particular, code should not assume that the alliance color will be available during constructor methods or `robotInit`, but it should be available by the time `autoInit` or `teleopInit` is called. FMS will set the alliance color automatically; when not connected to FMS, the alliance color can be set from the Driver Station (see :ref:`"Team Station" on the Operation Tab <docs/software/driverstation/driver-station:Operation Tab>`).
Note that there are three cases: red, blue, and no color yet. It is important that code handles the third case correctly because the alliance color will not be available until the Driver Station connects. In particular, code should not assume that the alliance color will be available during constructor methods or `Robot`, but it should be available by the time `autoInit` or `teleopInit` is called. FMS will set the alliance color automatically; when not connected to FMS, the alliance color can be set from the Driver Station (see :ref:`"Team Station" on the Operation Tab <docs/software/driverstation/driver-station:Operation Tab>`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be changed, as now it's redundant

@@ -42,8 +42,8 @@ To start, search above the ``unexpected error has occurred`` for the stack trace

.. code-block:: text

Error at frc.robot.Robot.robotInit(Robot.java:24): Unhandled exception: java.lang.NullPointerException
at frc.robot.Robot.robotInit(Robot.java:24)
Error at frc.robot.Robot.Robot(Robot.java:24): Unhandled exception: java.lang.NullPointerException
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect that this is not what the stack trace would look like

@spacey-sooty
Copy link
Contributor

Resolves #2679

@spacey-sooty
Copy link
Contributor

Remove usage of robotInit #2679 replaced with different constructor on the FRCDocs side, should I fork allwpilib too and change the examples there too before?

allwpilib has already removed this wpilibsuite/allwpilib#6623

@sciencewhiz sciencewhiz linked an issue Aug 6, 2024 that may be closed by this pull request
@sciencewhiz sciencewhiz added this to the 2025 milestone Aug 11, 2024
sciencewhiz added a commit to sciencewhiz/frc-docs that referenced this pull request Nov 11, 2024
Fixes wpilibsuite#2679
Supersedes wpilibsuite#2689
Does not touch RobotBuilder docs, as those are generated and need to be
updated by regenerating the code
Does not touch Python. robotpy/examples#120
sciencewhiz added a commit to sciencewhiz/frc-docs that referenced this pull request Nov 11, 2024
Fixes wpilibsuite#2679
Supersedes wpilibsuite#2689
Does not touch RobotBuilder docs, as those are generated and need to be
updated by regenerating the code
Does not touch stacktraces article, as the code and stacktraces need to
be updated together
Does not touch Python. robotpy/examples#120
jasondaming pushed a commit that referenced this pull request Nov 11, 2024
* Change usage of robotInit to Robot Constructor

Fixes #2679
Supersedes #2689
Does not touch RobotBuilder docs, as those are generated and need to be
updated by regenerating the code
Does not touch stacktraces article, as the code and stacktraces need to
be updated together
Does not touch Python. robotpy/examples#120

* Remove bad Override

* Remove additional reference
@sciencewhiz
Copy link
Collaborator

Superseded by #2841

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.

Remove usage of robotInit
3 participants