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.

Sign up for GitHub

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

Jump to bottom

linuxfs-pwm provider fixes for Pi5 operation. Default now pwmchip2, w… #339

Merged
merged 2 commits into from
Apr 8, 2024
Merged

linuxfs-pwm provider fixes for Pi5 operation. Default now pwmchip2, w… #339

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c2b149
linuxfs-pwm provider fixes for Pi5 operation. Default now pwmchip2, w…
taartspi Apr 1, 2024
ba13376
fine tune delay after export. Testing fast SSD failed at 25 ms, worke…
taartspi Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pi4j-core/src/main/java/com/pi4j/io/pwm/PwmBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ public Pwm initialize(Context context) throws InitializeException {
if(this.config.initialValue() != null){
try {
if(this.config.initialValue() <= 0){
this.off();
if(this.isOn()) {
this.off();
}
} else {
this.on(this.config.initialValue());
}
Expand Down
3 changes: 3 additions & 0 deletions pi4j-core/src/main/java/com/pi4j/util/Frequency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ public static float milliseconds(Number frequency){
*/
public static int getFrequencyFromNanos(Number nanoseconds){
int frequency;
if(nanoseconds.longValue() <= 0){
return(0);
}
long period = 1000000000; // NANOSECONDS PER SECOND;
frequency = Math.round(1000000000/nanoseconds.longValue());
return frequency;
Expand Down
3 changes: 2 additions & 1 deletion plugins/pi4j-plugin-linuxfs/src/main/java/com/pi4j/plugin/linuxfs/internal/LinuxPwm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public class LinuxPwm {
public static String DEFAULT_SYSTEM_PATH = "/sys/class/pwm";

/** Constant <code>DEFAULT_PWM_CHIP=0</code> */
public static int DEFAULT_PWM_CHIP = 0;
/** In Pi5 the chip is number 2 */
public static int DEFAULT_PWM_CHIP = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean, that it won't work on the Pi4 anymore? Shouldn't we be doing a check for the environment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So yes. When franks Context update expose the machine type i can set the chip number correctly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

test comment


protected final String systemPath;
protected final int chip;
Expand Down
7 changes: 6 additions & 1 deletion ...ns/pi4j-plugin-linuxfs/src/main/java/com/pi4j/plugin/linuxfs/provider/pwm/LinuxFsPwm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,18 @@ public Pwm initialize(Context context) throws InitializeException {
if(!pwm.isExported()) {
logger.trace("exporting PWM [" + this.config.address() + "]; " + pwm.getPwmPath());
pwm.export();
// Delay to allow the SSD to persist the new directory and device partitions
Thread.sleep(250);
Copy link
Member

Choose a reason for hiding this comment

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

Is this really needed? Maybe the export method need to do this check? What fails without this sleep?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the push last night the delay is 70 ms, a bit less pain. The app does pwm.on()x,y); If an export was not previously done, the code does an export and immediately call to set the period and calls to set the cycle. Setting the period fails with an access violation. In the export method we don't what will occur next and the delay fixes the exception. My assumption is the SSD has not completed the creation of the pwmX directory and contained files. I used this code very little on my Pi4 long ago and it worked. When we have system aware code in use I can verify that the Pi4 does not have a problem with no delay. Why 70 ms. 25 failed always, 30 worked nearly everytime. So although 35 might have worked with my fast SSD I doubled that to 70 assuming users with lesser quality SSD will need a greater delay.

Copy link
Member

Choose a reason for hiding this comment

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

The export will create a virtual directory in the kernel's memory space. This should not have anything to do with the attached storage. But it is certainly possible, that it takes the kernel a few milliseconds longer to configure the hardware for this, so i guess we'll keep these 70ms.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok Stay 70. I show my need to learn more on this. Sometime maybe i will do more kernel learning to understand what is syncronous and also how the storage is really used

} else{
logger.trace("PWM [" + this.config.address() + "] is already exported; " + pwm.getPwmPath());
}
} catch (java.io.IOException e) {
logger.error(e.getMessage(), e);
throw new InitializeException("Unable to export PWM [" + config.address() + "] @ <" + pwm.systemPath() + ">; " + e.getMessage(), e);
}
} catch (InterruptedException e) {
logger.error(e.getMessage(), e);
throw new InitializeException("Programmed delay failure, unable to export PWM [" + config.address() + "] @ <" + pwm.systemPath() + ">; " + e.getMessage(), e);
}

// [INITIALIZE STATE] initialize PWM pin state (via superclass impl)
super.initialize(context);
Expand Down