robotpy / mostrobotpy

Official Repository of python implementation of WPILib components
https://robotpy.github.io
Other
11 stars 14 forks source link

infinite value not propagated to NT4 topic properties #20

Closed gerth2 closed 1 year ago

gerth2 commented 1 year ago

MRE:

import wpilib
import ntcore

class MyRobot(wpilib.TimedRobot):
    def robotInit(self): 
        table = ntcore.NetworkTableInstance.getDefault().getTable("TestTable")

        topic = table.getDoubleTopic("testVal")
        self.pub = topic.publish(ntcore.PubSubOptions(
                                    sendAll=False, keepDuplicates=False))
        self.pub.setDefault(0.0)

        propValDes = float('-inf')
        topic.setProperty("doesntWork", propValDes)

        propValAct = topic.getProperty("doesntWork")
        print(propValDes)
        print(propValAct)

    def robotPeriodic(self):
        self.pub.set(wpilib.Timer.getFPGATimestamp())

if __name__ == '__main__':
    wpilib.run(MyRobot)

In Java, it was supported to pass inf and -inf as a value for the properties. In Python, a value of inf is returned from a paired get/set properties call. However, in NT4 clients (both the javascript one I've got and outlineviewer) the value comes back as null

image

virtuald commented 1 year ago

I can confirm that this occurs. Must be an issue with the wpi_json_type_caster.h in wpiutil? Though, there are tests that it can do floating point conversions, so there might be some other weird thing going on.

auscompgeek commented 1 year ago

Infinity isn't a valid JSON value; could it have something to do with that?

virtuald commented 1 year ago

If glass is able to display infinity from Java, then that means the C++ implementation supports it.

virtuald commented 1 year ago

This is a bug in the C++ implementation of ntcore. I added the following code to ntcore to ensure that only C++ code was setting the infinity value:

cls_Topic.def("setInfProp", [](nt::Topic &self) {
    self.SetProperty("doesn't work", -INFINITY);
  });

And then called it from python, and the property value was null on the remote side.

Reported as https://github.com/wpilibsuite/allwpilib/issues/5965

virtuald commented 1 year ago

See the discussion in 5965, but will likely close this as a wontfix. It turns out that the C++ implementation does not actually support infinity, so neither should have glass.

@gerth2 what happens in java when you pass in infinity?

sciencewhiz commented 1 year ago

What's the use case for passing infinity? The result of a bug or something desired?

gerth2 commented 1 year ago

Lemme check tomorrow. I believe it resolved to inf somehow on the java side

Our use case was encoding min/max range for an NT value used as a calibration. Some had a finite range, others had infinity for those limits.

gerth2 commented 1 year ago

Looking at our behavior last year, and checking in simulation:

image

Java put Infinity in.

image

gerth2 commented 1 year ago

Looked at the other issue - I'm ok if this becomes a wontfix too - I can work around this in other ways, and it looks like I only got lucky that it worked in past years. Thanks for the digging!

virtuald commented 1 year ago

The Infinity values in your example are strings (because they have quotes around them), which would work fine here as well if you elected to do that. The C++ JSON library encodes infinity as null instead of a string.