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

MultiConfigurePC not usable? #582

Open
RobertPHeller opened this issue Oct 26, 2021 · 15 comments
Open

MultiConfigurePC not usable? #582

RobertPHeller opened this issue Oct 26, 2021 · 15 comments

Comments

@RobertPHeller
Copy link
Contributor

Since the standard thing for Gpio * classes is to use the GpioWrapper class and since the GpioWrapper class does not allow changing direction, this means that one cannot have Gpio Lines that can be configured as input or output, this means that MultiConfigurePC cannot be used. Either it should be removed or GpioWrapper needs to be replaced or possibly something else.

Using the MultiConfigurePC config causes a run-time crash due to the HASSERTs in the set_direction() method of the GpioWrapper class.

@atanisoft
Copy link
Collaborator

I've used the MultiConfigurePC object successfully on the ESP32-S2 (though it should work on others), I encountered the same limitation and modified the Esp32Gpio code to not use GpioWrapper.

@RobertPHeller
Copy link
Contributor Author

OK, I will look at Esp32Gpio and see if I can make a similar modification to LinuxGpio...

I wonder: if all of the *Gpio implementations are eventually modified to not use GpioWrapper, what happens to GpioWrapper? -- I wonder if GpioWrapper's set_direction() is plain wrong -- eg why does it have the "restriction" against changing direction?

@atanisoft
Copy link
Collaborator

I believe most of the Gpio implementations have already moved off GpioWrapper already.

@RobertPHeller
Copy link
Contributor Author

OK, so GpioWrapper is effectively depreciated?

@atanisoft
Copy link
Collaborator

I can't say for certain on that, it is still used by a handful of target (STM32 being one).

@balazsracz
Copy link
Collaborator

balazsracz commented Oct 26, 2021 via email

@atanisoft
Copy link
Collaborator

std::enable_if is only available in C++14 which is not available by default in arduino-esp32 (possibly other platforms). We will need a fallback option when using C++11 (which is really old!)

@balazsracz
Copy link
Collaborator

balazsracz commented Oct 26, 2021 via email

@atanisoft
Copy link
Collaborator

Ahh, I landed on a diff page that stated C++14. If we can craft the necessary magic we can certainly go that route.

@RobertPHeller
Copy link
Contributor Author

I have recoded LinuxGpio to not use GpioWrapper (using Esp32Gpio as a model), but I am getting a compile error:

/include -I. -I/home/heller/RRCircuits/MegaIOOpenMRN --std=c++11 -MD -MF compile_cdi.d /home/heller/openmrn/src/openlcb/CompileCdiMain.cxx
In file included from ./Hardware.hxx:4,
from ./config.hxx:10,
from /home/heller/openmrn/src/openlcb/CompileCdiMain.cxx:6:
/home/heller/openmrn/src/os/LinuxGpio.hxx:176:46: error: template definition of non-template ‘const LinuxGpio<PIN_NUM> LinuxGpio<PIN_NUM>::instance_’
const LinuxGpio<PIN_NUM> LinuxGpio<PIN_NUM>::instance_;
^~~~~~~~~

Code is pushed to my fork: https://github.com/RobertPHeller/openmrn/blob/LinuxGpio-no-GpioWrapper/src/os/LinuxGpio.hxx

@atanisoft
Copy link
Collaborator

https://github.com/RobertPHeller/openmrn/blob/LinuxGpio-no-GpioWrapper/src/os/LinuxGpio.hxx#L93 declares the template type as "int".
https://github.com/RobertPHeller/openmrn/blob/LinuxGpio-no-GpioWrapper/src/os/LinuxGpio.hxx#L175 defines the type as "gpio_num_t" (define to unsigned int).

I'd suggest drop all usages of gpio_num_t and use int or even uint8_t if you can fit the IO pin count in that size.

@atanisoft
Copy link
Collaborator

also it would be better to use typedef instead of define for types...

@RobertPHeller
Copy link
Contributor Author

OK, all fixed now, ready for a pull request once I test things.

@balazsracz
Copy link
Collaborator

balazsracz commented Oct 26, 2021 via email

@RobertPHeller
Copy link
Contributor Author

Yeah, a bit of a copy/paste snafu...

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

No branches or pull requests

3 participants