Skip to content

Conversation

kamilwierzbicki
Copy link

Issue: #287

Some of the PassThru devices report an error when an invalid bit is set in the flags parameter.
Currently the bit 6 is set but it should be zeroed.

@pylessard
Copy link
Owner

Not sure changing the definition of the constant is a proper fix. Should be zeroed at the usage point. Also, does this limitation apply to your use case only or all use case?

@kamilwierzbicki
Copy link
Author

Constant value needs to be changed because it's invalid according to specification
image
Only bit 8 defines the length of CAN identifier so setting up the bit 7 for 11 bit length CAN identifiers caused the problem.

@pylessard
Copy link
Owner

Then why do you change the constant value to 0?
@kirya-dev, can you get involved? I never used J2534

@kamilwierzbicki
Copy link
Author

This constant contains the value that needs to be set to get 11 bit CAN identifier. According to specification bit 8 specifies the CAN identifier length. If the CAN identifier is 29 bit then bit 8 should be set (0x400). Otherwise it should be cleared (0x000). This constant value can be deleted at all but I left it to keep it more readable.

@kirya-dev
Copy link
Contributor

Otherwise it should be cleared (0x000)

No. Should be cleared only 8 bit. BUT i think better if const ISO15765_CAN_ID_11 will be deleted.
And modify flags assigns as shown here:

        self.txConnectFlags = 0
        # Determine mode ID29 or ID11
        if txid >> 11:
            self.txConnectFlags |= TxStatusFlag.ISO15765_CAN_ID_29.value
        self.txFlags = self.txConnectFlags | TxStatusFlag.ISO15765_FRAME_PAD.value

@pylessard
Copy link
Owner

@kirya-dev : It sure feels like adding the 29 bits flag if required makes more sense.
But looking at your solution, what if I want a 29bits address that has a values smaller than 0x7FF?

@kirya-dev
Copy link
Contributor

what if I want a 29bits address that has a values smaller than 0x7FF?

I see topic start from another problem - bit 6 should be zerowed. No related with ID len. Related with FRAME_PAD flag.

@kamilwierzbicki kamilwierzbicki force-pushed the fix_passthruconnect_flags branch from 27af056 to df6a583 Compare August 28, 2025 07:58
@kamilwierzbicki
Copy link
Author

kamilwierzbicki commented Aug 28, 2025

Updated according to your comments.
@pylessard Your observation may be resolved in another CR. There is already an issue created for that.
#286

@pylessard
Copy link
Owner

@kirya-dev : I still find this PR weird. Will follow your call for merge.

@kirya-dev
Copy link
Contributor

I can approve condition style shown in my message above.

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

Successfully merging this pull request may close these issues.

3 participants