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

removeListener? #91

Open
fabdrol opened this issue Dec 9, 2020 · 9 comments
Open

removeListener? #91

fabdrol opened this issue Dec 9, 2020 · 9 comments

Comments

@fabdrol
Copy link

fabdrol commented Dec 9, 2020

AFAIK there's no way to call removeListener on a channel. This poses some questions:

  • When I create a raw channel, and it stops (onStopped event), are the listeners garbage collected or do they persist? Can I safely set the variable holding the reference to the result of createRawChannel to undefined, then create a new raw channel, without leaking memory?

  • If I want to de-register a listener for a running channel, and register a new listener, how do I handle that?

@sebi2k1
Copy link
Owner

sebi2k1 commented Dec 12, 2020

The listener is associated to the channel objects itself regardless of its state.

Adding/removing listeners after starting it was not considered, let me check.

@aufabdulnafea
Copy link

Hello,

Thank you very much for the great library. However, I need to be able to remove the listener from a RawChannel, as I am implementing the UDS protocol. I want to add a listener for each UDS request and then remove it when the response is completed or if it's interrupted.

@aufabdulnafea
Copy link

this is the uds function if it helps.

uds(id: number, data: number[]): Promise<IUdsResult> {
    // this._validateData(data);

    data = data.concat(Array(8).fill(0x00)).slice(0, 8);
    const service = data[1];
    const expectedServiceId = 0x40 + service;

    if (data.length > 8) throw new Error("the data length should not exceed 8");

    // add padding (0x00) if the length of the data is less than 8

    return new Promise((resolve, reject) => {
      let responseLength = 0;
      let response: number[] = [];

      let nextSequentialNumber = 0;

      this._channel.addListener("onMessage", (message: any) => {
        if (message.id !== 0x7d1) return;
        const data = [...message.data];

        const frameType = (data[0] & 0xf0) >> 4;
        if (frameType > 2 || frameType < 0) return;

        // single frame (SF)
        if (frameType === 0) {
          if (data[1] !== expectedServiceId) return;
          responseLength = data[0] & 0xf;
          response = data.slice(1, Math.min(responseLength + 2, 8));
          return resolve({
            data: response,
            parsed: this._udsParser(response),
          });
        }

        // first frame (FF)
        else if (frameType === 1) {
          if (data[2] !== expectedServiceId) return;
          responseLength = ((data[0] & 0xf) << 8) | data[1];
          response = data.slice(2, 8);
          nextSequentialNumber = 1;
          this.send(id, [0x30, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]);
        }

        // Consecutive Frame (CF)
        else if (frameType === 2) {
          const currentSequence = data[0] & 0xf;
          if (nextSequentialNumber !== currentSequence)
            return reject("the sequence of the request is wrong");

          nextSequentialNumber += 1;
          if (nextSequentialNumber > 0xf) nextSequentialNumber = 0;

          response = [
            ...response,
            ...data.slice(1, Math.min(responseLength - response.length + 1, 8)),
          ];

          if (responseLength - response.length === 0) {
            return resolve({
              data: response,
              parsed: this._udsParser(response),
            });
          }
        }
      });
      this.send(id, data);
    });
  }

@juleq
Copy link
Contributor

juleq commented Oct 13, 2023

I do not speak for the maintainer but it would be effort to add removal since it is in the native part of the code.

Could you elaborate on your use case? Why would retaining an active/inactive flag for the listener not do the trick? If you find it to be inactive, you just early return.

@aufabdulnafea
Copy link

Hi @juleq thanks for you response, you suggestion could do the trick, I still need to change the expected ID, but I thought it would be cleaner to remove the listener. The Signal according to the documentation has a removeListener method, but it is not clear to me if I can use the signals in my case

@juleq
Copy link
Contributor

juleq commented Oct 14, 2023

Since you do not use signal based communication but a higher level one, using a (fake) signal would be kind of hacky.

I would stick to the onMessage listener.

@juleq
Copy link
Contributor

juleq commented Oct 14, 2023

If, at some point, you want to venture into playing with the native code, I am sure a PR with a straightforward implementation of removeListener on the channel would be accepted.

@Kheirlb
Copy link

Kheirlb commented Feb 24, 2024

I think I am running into a possible similar issue. I would like to be able to dynamically switch between can0 and vcan0. I assume that is not an option?

export const getDatabaseService = (canInterface: string): DatabaseService => {
  const network = parseNetworkDescription("kr.kcd");
  const channel = createRawChannel(canInterface);
  channel.start();
  const kr_bus = new DatabaseService(channel, network.buses["kr"]);
  // setup listeners
  return kr_bus;
}

// initial setup on vcan0
let gKrBus = getDatabaseService("vcan0")
// later a switch to can0
gKrBus = getDatabaseService("ccan0")

Unfortunately, I was still seeing data from the callback functions setup during the vcan0 step. I can restructure to have some sort of flag as juleq suggested.

@sebi2k1
Copy link
Owner

sebi2k1 commented Feb 26, 2024

Mh, the way you built should actually work. What you can’t do, at least which interferes with these signal/database and CAN in general, is the send the same message from multiple nodes.

can you share more details what is happening?

As long as you have dedicated database per channel they work concurrently and depending on your network configuration (can0 and can1) are connected to the same bus you should be able to exchange signals between both CAN interface. But again, never send the same frames from different node. That usually not the way CAN and those node/bus concepts are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants