getting WARNING: Invalid parameter name

[2bndy5]

I'm now getting warnings saying

GitHub\RF24\docs\sphinx\classRF24.rst:6: WARNING: Invalid parameter name: '_cepin'
GitHub\RF24\docs\sphinx\classRF24.rst:6: WARNING: Invalid parameter name: '_cspin'
GitHub\RF24\docs\sphinx\classRF24.rst:6: WARNING: Invalid parameter name: '_spi_speed'
GitHub\RF24\docs\sphinx\classRF24.rst:7: WARNING: Invalid parameter name: '_spi_speed'

The only commonality that these parameters have is that they're used as verbatim parameter names in multiple functions ( in overloaded c'tor and Arduino style begin()), and each name begins with a _. I might be misunderstanding the problem here, but I doubt that the _ is the culprit (removing the _ didn't prevent the warning).

EDITED: better diagnosis in first comment below

I'm guessing this warning should be saying "Non-unique parameter name" given that it is generated from

sphinx-immaterial/sphinx_immaterial/cpp_domain_fixes.py

Lines 728 to 740 in 163fe76

	# Determine the number of unique declarations of this parameter
	unique_decls: Dict[str, Tuple[int, docutils.nodes.Element]] = {}
	for i, sig_param_nodes in enumerate(sig_param_nodes_for_signature):
	desc_param_node = sig_param_nodes.get(param_name)
	if desc_param_node is None:
	continue
	decl_text = desc_param_node.astext().strip()
	unique_decls.setdefault(decl_text, (i, desc_param_node))
	if not unique_decls:
	logger.warning(
	"Invalid parameter name: %r", param_name, location=param_node
	)
	continue

However, I would argue that re-using parameter names for multiple functions is an accepted practice. So, we shouldn't raise warnings about it (which would cause RTD builds to fail fast).

[2bndy5]

I've been looking into this, and I think there's something funky going on with c'tors. I think it does 2 passes:

1. 1 successful pass for each c'tor function signature
2. 1 pass for the class signature which triggers the warnings for all parameters in each c'tor

This kinda makes sense why the rendered docs behave as expected.

My initial guess about duplicate param names (for different functions) doesn't seem to be the problem. I still think this warning could be a bit more descriptive like

WARNING: No corresponding parameter description for 'param_name'

---

Just as an experimental fix, I've wrapped

sphinx-immaterial/sphinx_immaterial/cpp_domain_fixes.py

Lines 890 to 892 in 163fe76

	_cross_link_parameters(
	app=self.env.app, domain=domain, content=self.contentnode, symbols=symbols
	)

into

        if self.contentnode.parent["desctype"] != "class":
            _cross_link_parameters(
                app=self.env.app,
                domain=domain,
                content=self.contentnode,
                symbols=symbols
            )

Now the docs render as expected and there are no warnings.

[jbms]

Do I understand correctly that you have something like:

.. cpp:class:: MyClass

   :param _cepin: Description goes here.

   .. cpp:function:: MyClass(int _cepin)

     Constructor.

That is, you are trying to document a constructor parameter within the description of the class itself?

The current implementation assumes that you would only document constructor parameters within the description of the constructor, not the containing class.

Note that we don't want to disable cross linking of all parameters for classes, because classes can have template parameters (just not function parameters), and _cross_link_parameters handles both types of parameters.

[2bndy5]

Yes, but the param descriptions are part of the c'tor's function directive (not the class directive).

.. cpp:class:: RF24

    .. doxygenfunction:: RF24::RF24 (uint16_t _cepin, uint16_t _cspin, uint32_t _spi_speed=RF24_SPI_SPEED)
    .. doxygenfunction:: RF24::RF24 (uint32_t _spi_speed=RF24_SPI_SPEED)

[2bndy5]

Based on your feedback to #70 I think a better worded warning would be:

No parameter found in signature for 'param_name'

[jbms]

Agreed on the better warning name.

Can you reproduce the constructor issue without using breathe? There may well be a bug but it would be easier to understand just using the plain cpp domain directives.

[2bndy5]

.. cpp:class:: RF24

    .. cpp:function:: RF24::RF24(uint16_t _cepin, uint16_t _cspin, uint32_t _spi_speed=RF24_SPI_SPEED)
        
        :param _cepin: The pin attached to Chip Enable on the RF module
        :param _cspin: The pin attached to Chip Select (often labeled CSN) on the radio module.
        
            - For the Arduino Due board, the `Arduino Due extended SPI feature <https://www.arduino.cc/en/Reference/DueExtendedSPI>`_
              is not supported. This means that the Due's pins 4, 10, or 52 are not mandated options (can use any digital output pin) for the radio's CSN pin.
        :param _spi_speed: The SPI speed in Hz ie: 1000000 == 1Mhz
            
            - Users can specify default SPI speed by modifying :c:macro:`RF24_SPI_SPEED` in :file:`RF24_config.h`
            
              - For Arduino, the default SPI speed will only be properly configured this way on devices supporting SPI TRANSACTIONS
              - Older/Unsupported Arduino devices will use a default clock divider & settings configuration
              - For Linux: The old way of setting SPI speeds using BCM2835 driver enums has been removed as of v1.3.7

    .. cpp:function:: RF24::RF24(uint32_t _spi_speed=RF24_SPI_SPEED)

        :param _spi_speed: The SPI speed in Hz ie: 1000000 == 1Mhz
            
            - Users can specify default SPI speed by modifying :c:macro:`RF24_SPI_SPEED` in :file:`RF24_config.h`
            
              - For Arduino, the default SPI speed will only be properly configured this way on devices supporting SPI TRANSACTIONS
              - Older/Unsupported Arduino devices will use a default clock divider & settings configuration
              - For Linux: The old way of setting SPI speeds using BCM2835 driver enums has been removed as of v1.3.7

Pure RST does reproduce the warnings. I am also seeing the warnings' RST line number correspond to the end of the cpp:function directives. Is there something else I can provide (like pformat() output)?

[2bndy5]

I noticed the tooltip has 3 RF24:: scope resolutions (I'd expect there should only be 2).

[2bndy5]

using

.. cpp:class:: RF24
    .. cpp:function:: RF24(...)

reduces the RF24:: scope resolutions to 2, but it still produces the warnings.

[jbms]

I will look into the parameter issue.

The RF24::RF24::RF24 issue is that since you are defining it within the RF24 class, it is assumed to be within that scope. Instead you should just declare the constructors as .. cpp:function:: RF24(...). I don't think my changes have any impact on that issue.

If you just want it to display as RF24::RF24 that would probably require some other change.

[jbms]

Okay, the issue is actually that _add_parameter_documentation_ids when run on the class is finding the term nodes within the nested function declaration --- I will have to fix that.

[2bndy5]

Dude, I've been probing that function's code for hours to understand what it is actually doing. At best, I found a greater appreciation for your work...

[jbms]

That is a sign that it needs better comments I guess.