Double defined line object, cannot call getValue or setValue funciton.

[TaylorIsAGoodboy]

Hi, Leonardo
I defined line twice, then it occured "Error: Unable to get value for this line".
Is there some way to this error.
I make a nodered node which based on your node-libgpiod library. The first time is good，then move debug node and deploy again, error occurs. I think the reason it i double defined line object.

[sombriks]

Hello @TaylorIsAGoodboy !

you mean if, for some reason, same line gets defined twice during code execution, it will fail?

please provide a small project sampling the situation so i can try it on the hardware available here.

[TaylorIsAGoodboy]

I wrote a nodered node based on your library. I depoyed flow, it worked ok. But when I modified something, and depoyed again it shows error. Error may be "unable to get value for this line", "Segment fault" or "double free".

Url of nodered node is https://github.com/TaylorIsAGoodboy/nodered-gpiod-node

[TaylorIsAGoodboy]

Hello @TaylorIsAGoodboy !

you mean if, for some reason, same line gets defined twice during code execution, it will fail?

please provide a small project sampling the situation so i can try it on the hardware available here.

You can run the script below at two different term.

const { version, Chip, Line } = require("node-libgpiod");

let chip = new Chip(2);
let line = new Line(chip, 7); 

line.requestOutputMode();

const blink = function () {
  // avoid early gc
  this.chip = chip
  this.line = line
  let val = this.line.getValue();
  this.line.setValue(1-val);
  setTimeout(blink, 1000);
};

setTimeout(blink, 1000);

Do we have a way to release the line before we call new Line() to define the line?

[sombriks]

Hello @TaylorIsAGoodboy, i just merged a contributed patch and will make another release!
Once it done please let me know if it is solved.

[dmitrydvorkin]

Hello, @TaylorIsAGoodboy

Do we have a way to release the line

obviously, yes.
In my gpio module for nodeRed (will be published within 2 days) I am doing like this:

node.on('close', function (removed, done) {
  node.line.release()
  done()
})

It is described in documentation:
https://nodered.org/docs/creating-nodes/node-js#closing-the-node

It helps alot.

BTW, if you'll do
let line = new Line(chip, 7);
line = new Line(chip, 7);

obviously, descrtructor for the first object has to be called at the second line right before "line" object will be set to the new value. I think, It fails at desctructor. Check it with my update. If you'll experience the same problem just enable the debugging at chip.hh and see the calls sequence

[noctarius]

I guess it'd be possible to cache the line objects internally, but that would cause plenty of other issues. From my perspective the issue is not two line objects, but both calling requestXMode. Is there any way in node-red to cache / provide objects to be shared between multiple node instances?

My guess would be, that the event handler for close runs to late (see nodejs event handling) and the second node is executed before the close event was handled (though the first line object isn't released yet).

[dmitrydvorkin]

Dear @noctarius !
You commit is very good. But things like
getNumberOfLines, getChipName, getChipLabel
are libgpiod-specific :) We could get rid of libgpiod dependency and simply use native linux functions. It could even simplify the coding :)

also some things are useless. For example, if you get number of pins = 200 it means nothing - 80% of them could be preallocated and not available.
chipName and chipLabel are SoC- and driver-specific. I suspect, most implementations are simply returns just the pinmux driver name.

The really useful thing that should be added is the way to find out the list of all GPIO chips available.
Most SoCs has 1 GPIO chip. But some has 2 or 4, each contains from 16 to 32 or 64 pins.

[TaylorIsAGoodboy]

Hello @TaylorIsAGoodboy, i just merged a contributed patch and will make another release! Once it done please let me know if it is solved.

OK， I will test it tommorrow. Thank you for your efforts!

[TaylorIsAGoodboy]

Hello @TaylorIsAGoodboy, i just merged a contributed patch and will make another release! Once it done please let me know if it is solved.

Hi @sombriks , the issue still existed.

[TaylorIsAGoodboy]

Dear @noctarius ! You commit is very good. But things like getNumberOfLines, getChipName, getChipLabel are libgpiod-specific :) We could get rid of libgpiod dependency and simply use native linux functions. It could even simplify the coding :)

also some things are useless. For example, if you get number of pins = 200 it means nothing - 80% of them could be preallocated and not available. chipName and chipLabel are SoC- and driver-specific. I suspect, most implementations are simply returns just the pinmux driver name.

The really useful thing that should be added is the way to find out the list of all GPIO chips available. Most SoCs has 1 GPIO chip. But some has 2 or 4, each contains from 16 to 32 or 64 pins.

Hi @dmitrydvorkin, If your nodered node was ready, please share me the url, thank you.
My node still has issues, when i add release() funciton in 'close' event, it shows timeout.

[dmitrydvorkin]

here is a link:
https://github.com/s5z6/node-red-contrib-libgpiod

https://www.npmjs.com/package/node-red-contrib-libgpiod
https://flows.nodered.org/node/node-red-contrib-libgpiod

[sombriks]

@TaylorIsAGoodboy i am building a test case for your specific issue, don't know if it helps but create several instances of lines are ok, but for some reason requestOutputMode more than once kicks the Error. still early steps here but i'll investigate more.

[sombriks]

we might just need to know current line direction before request intput or output mode
https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/lib/helpers.c?h=v1.6.x

let's look for some useful call in the source.

[TaylorIsAGoodboy]

Yeah, but it may be done in requestMode funtion to avoid duplicate settings. Taylor Lee 李远朝 Software Engineer 软件工程师 | China E-mail: ***@***.*** | Mobile: +86 138 9185 6324 深圳市瑞科慧联科技有限公司 | Shenzhen RAKwireless Technology Co.,Ltd. 深圳市南山区桃源街道平山一路新视艺创客公园B栋506室 Room 506, Bldg B, New Compark, Pingshan First Road, Taoyuan Street, Nanshan District, Shenzhen, China www.RAKwireless.com | Twitter | YouTube | 微信公众号/官方微博：RAK瑞科慧联 From: Leonardo Silveira Date: 2022-04-19 10:10 To: sombriks/node-libgpiod CC: Taylor; Mention Subject: Re: [sombriks/node-libgpiod] Double defined line object, cannot call getValue or setValue funciton. (Issue #4) we might just need to know current line direction before request intput or output mode https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/lib/helpers.c?h=v1.6.x let's look for some useful call in the source. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[sombriks]

Yes, totally agree!

[noctarius]

I think the issue is a different one. From my perspective it's not about the direction, but you cannot reserve a line twice, that said, the previous reservation needs to be released before a new one can be acquired. It's luckily not a US airline carrier overbooking their flights ;-)

[dmitrydvorkin]

@noctarius @sombriks !

if you'd run the code in debug mode (I added it in my commit) you can see:
../src/chip.cc New():31
../src/chip.cc New():33
../src/chip.cc Chip():15
../src/chip.cc Chip():17 0x207b2e8
../src/chip.cc New():36 0x212e198
../src/chip.cc New():38 0x212e198
../src/chip.cc New():48
../src/line.cc New():37
../src/line.cc New():39
../src/line.cc Line():21
../src/chip.cc getNativeChip():52 0x207b2e8
../src/line.cc Line():23 0x2094d70
../src/line.cc New(12):43 0x207b2b8
../src/line.cc New(12):45 0x207b2b8->0x2094d70
../src/line.cc New():55
../src/line.cc New():37
../src/line.cc New():39
../src/line.cc Line():21
../src/chip.cc getNativeChip():52 0x207b2e8
../src/line.cc Line():23 0x2094d70
../src/line.cc New(12):43 0x207b2d0
../src/line.cc New(12):45 0x207b2d0->0x2094d70
../src/line.cc New():55
../src/line.cc ~Line():28
../src/line.cc ~Line():30
../src/line.cc ~Line():32
1.4.3
../src/line.cc requestOutputMode():93
../src/line.cc requestOutputMode():108 0x207b2d0
../src/line.cc getNativeLine():126 0x2094d70
/home/root/exp/node-red-contrib-libgpiod/xx.js:9
line.requestOutputMode();
^

Error: ::requestOutputMode() failed

---

do you see
../src/line.cc New(12):45 0x207b2b8->0x2094d70
../src/line.cc New(12):45 0x207b2d0->0x2094d70
?
I've created two Line objects with same pin#.
Yes, it is two different objects, but THE struct gpiod_line * pointer returned by gpiod_chip_get_line() IS THE SAME?
Why?
We have same Chip object for both Line objects and please, take a look at code of the gpiod_chip_get_line() there:
https://github.com/brgl/libgpiod/blob/cc23ef61f66d5a9055b0e9fbdcfe6744c8a637ae/lib/core.c#L291

It simply returns the reference to previously allocated gpiod_line struct.
And right before we do .getValue() or .setValue() the destructor for the first Line object is called.
Destructor closes the line handle and free its resources inside LibGPIOD.
so any next calls that is using the reference to this gpiod_line * FAIL.

Obviously we can't call gpiod_line_close_chip(line); till we have copy of this object in some other Line instance.

How to do that correctly in NodeJS? I don't know.

I'm thinking about some reference counter or thomething like this. This is the task for man who have good experience in NodeJS internals.

[noctarius]

Do you have some test code? Normally, to prevent an object from being GC'd (and hence deconstructed) you need to hold a hard reference to the object. For example by keeping a module local cache of all loaded chips (or by keeping it referenced in some object) and export a function which uses that cache. No need to sync for concurrency, since nodejs doesn't have concurrency (do to the event loop).

This is basically what I do with our Port classes. And when creating a port, I reuse previously chip instances. That way you prevent the chip object from not being referenced and garbage collected.

class Port {
  #chip
  #line

  constructor(chip: Chip, offset: number) {
    this.#chip = chip; // DO NOT DELETE to prevent it from being GC'd
    this.#line = newLine(chip, offset);
  }
}

function newPort(...) {
  const chips: Array<Chip> = [];
  let chip = chips[gpiochip];
  if (chip === undefined) {
    chip = newChip(gpiochip);
    chips[gpiochip] = chip;
  }
  return newPort(chip, gpio)
}

[dmitrydvorkin]

test code:

const { version, Chip, Line } = require("node-libgpiod");

const chip = new Chip(0);
var line = new Line(chip, 12); // led on GPIO12
line = new Line(chip, 12); // led on GPIO12
let count = 10;

console.log(version());
line.requestOutputMode();

const blink = () => {
  if(count){
    line.setValue(count-- % 2);
    setTimeout(blink,1000);
  } // else line.release(); 
  // not needed, libgpiod releases resources on process exit  
};

setTimeout(blink,1000);

@noctarius , I think it should be fixed with
ObjectReference
https://github.com/nodejs/node-addon-api/blob/main/doc/object_reference.md
or
https://stackoverflow.com/questions/34175830/how-to-implement-reference-counting-in-javascript

[noctarius]

Haven't tested / really looked at it yet. Just making sure, NaN is a different C API adapter than Napi. Not sure you can mix and match them, also not really a nodejs developer :)

edit: seems like https://github.com/nodejs/nan/blob/main/doc/persistent.md NaN offers a similar functionality with NaN::Persistent

[TaylorIsAGoodboy]

test code:

const { version, Chip, Line } = require("node-libgpiod");

const chip = new Chip(0);
var line = new Line(chip, 12); // led on GPIO12
line = new Line(chip, 12); // led on GPIO12
let count = 10;

console.log(version());
line.requestOutputMode();

const blink = () => {
  if(count){
    line.setValue(count-- % 2);
    setTimeout(blink,1000);
  } // else line.release(); 
  // not needed, libgpiod releases resources on process exit  
};

setTimeout(blink,1000);

@noctarius , I think it should be fixed with ObjectReference https://github.com/nodejs/node-addon-api/blob/main/doc/object_reference.md or https://stackoverflow.com/questions/34175830/how-to-implement-reference-counting-in-javascript

Yes, it may be the reason. I find the c libray to call ioctl to operate gpio. Is it difficulty writing a pure nodeJs library?
I find there is a python library which doesnot depend c library. The address is https://pypi.org/project/gpiod/.

And there is a ioctl library, we may create a pure js libgpiod based on this.
The address is https://www.npmjs.com/package/ioctl.

[dmitrydvorkin]

with C library and direct ioctl commands you'll have same issue, but with "int fd" (file descriptor).
if you're not able to have only 1 NodeJS object for 1 line, you'll have "race condition" problem.
So line object has to be unique globally.
Same rule for Chip object.

there are should be the way to do it in NodeJS

I have an idea:
Create one global "Lines" object and use it as sigletone.
When you want top access to certain line you should call
Lines.setValue( linenum, val)

counstructor should be
Lines( Chip &_chip).

object Chips should be signletone too with method to get certain chip, like:
let chip = Chips.get( "gpiodev0")

[sombriks]

i think NAN will help us moving to global/persistent instances
https://github.com/nodejs/nan#persistent-references

will try something later

[dmitrydvorkin]

it should be like this:
every /dev/gpiochip* should have 1 Lines object.
when you want to access certain line, you just call
Lines.somefunc( pinnum, params)

thats all!

[TaylorIsAGoodboy]

i think NAN will help us moving to global/persistent instances https://github.com/nodejs/nan#persistent-references

will try something later

OK, If ok, please tell me. I will test. Thank you.

[sombriks]

added a testcase for the double definition issue.

basically we can define and redefine a line as many times we want as long we release the line.

pin is a limited resource so i think it makes sense.

and finally we have a few instructions on how to run the gpio-sim, that might help to understand the issue furthermore.

for now i am closing this issue because error handling either got better or libgpiod itself evolved since then.