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

hal.port.digital_out causing a hang #42

Closed
5ocworkshop opened this issue Jul 24, 2021 · 6 comments
Closed

hal.port.digital_out causing a hang #42

5ocworkshop opened this issue Jul 24, 2021 · 6 comments

Comments

@5ocworkshop
Copy link

5ocworkshop commented Jul 24, 2021

I'm working on a new plugin that uses that uses the Output_Aux lines but when I address them it hangs the code.

I am able to use standard Arduino pinMode and digitalWrite commands to write to the pins and they behave as expected - so the pathway is correct electrically.

Is this the correct syntax to have the line go high?

hal.port.digital_out(Output_Aux0, true)

(EDIT: I thought I think I found mistake, the comment above the point referes to true but the actual command wants a bool and it says on. However "on" is not defined but "On" and "Off" are defined. Exploring now.)

Also, I haven't located it yet, but I assume the output pins get set to output mode somewhere or should I be initializing them?

@5ocworkshop
Copy link
Author

I thought perhaps there was something unique in the bool situation that I wasn't familiar with but I have cconfirmed that none of the pairs of true/false, 1/0 or On/Off work. Yes, I know they should all evaluate to being equivalent, just trying to poke it to see if I could get a different result.

Are there setup steps required before the ports are available via the HAL commands?

@5ocworkshop
Copy link
Author

5ocworkshop commented Jul 24, 2021

Poked around a bit more. I had originally been testing in my new module but moved the testing to the coolant control function so I could use M7 and M9 to trigger the event. I noticed I hadn't included driver.h in the coolant_control.c, so I added that and I can see that Output_Aux0 evaluates to "82". However, the result is the same. The command hangs the system.

I am manually setting the pin the output before running the command, until I can confirm the init occurs elsewhere.

In driver.c I can see that .pin = AUXOUTPUT0_PIN correctly resolves to the pin I expect it to (derived from my_machine_map.h).

@5ocworkshop
Copy link
Author

Looking at the syntax in fan.c I thought this should work:

hal.port.digital_out(0, true);
delay(1000);
hal.port.digital_out(0, false);

But it also hangs the board.

I'm also a little bit confused that we are using integers to refer to the ports when we have the whole xbar with human readable names. Perhaps I am misunderstanding how the HAL is intended to work, but I would have thought doing the lookup by name would improve readability?

@5ocworkshop
Copy link
Author

Mystery solved. My my_machine_map.h was missing the line:

#define HAS_IOPORTS

It might be good to have an error check when calling digital_in and digital_out to ensure the ioports.c module was compiled in and output to console gracefully. I am not familiar enough with the code to offer a patch at the moment but I hope to get up to speed and contribute going forward.

@terjeio
Copy link
Contributor

terjeio commented Jul 24, 2021

It might be good to have an error check when calling digital_in and digital_out to ensure the ioports.c module was compiled in and output to console gracefully.

When using ioports you have to check if ports are available before calling the API directly or use the default M-codes (M62-M67) as those returns an error if not available.
If the API is called from a plugin the ports should be "claimed" by decrementing number of available ports so that other plugins and/or default M-codes do not access them. An example is the init code for the fans plugin:

    if(hal.port.num_digital_out >= FANS_ENABLE) {    // check availability 


        hal.port.num_digital_out -= FANS_ENABLE;     // decrement number of available ports so that other plugins cannot see or use it
        base_port = hal.port.num_digital_out;            // save the port number of the claimed port(s)
...

I am not familiar enough with the code to offer a patch at the moment but I hope to get up to speed and contribute going forward.

For this better documentation is required? Different drivers and even different board maps will not neccessarily support the same number of ports or even any at all (or they may be claimed by other plugins earlier in the startup sequence). Never assume that ports are available and provide feedback if not.

@5ocworkshop
Copy link
Author

For this better documentation is required? Different drivers and even different board maps will not neccessarily support the same number of ports or even any at all (or they may be claimed by other plugins earlier in the startup sequence). Never assume that ports are available and provide feedback if not.

I hadn't seen this when you posted it but around the same time I saw how the Fans plugin was handling things and I followed that structure, so I do have the port decrementing and error handling included now. Thanks for flagging it.

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

2 participants