milhead2 / python-j1939

Breakout of j1939 from inside the python-can package
GNU Lesser General Public License v3.0
38 stars 20 forks source link

18 bit PGN changes break 16 bit masking #22

Open brearl opened 1 year ago

brearl commented 1 year ago

The 18 bit PGN mask changes break the 16 bit masking and don't appear to work correctly for 18 bit PGNs. The patch below appears to correct this:

diff --git a/j1939/pgn.py b/j1939/pgn.py index 24611c1..4305ffc 100644 --- a/j1939/pgn.py +++ b/j1939/pgn.py @@ -42,9 +42,9 @@ class PGN(object):

 @value.setter
 def value(self, value):

@@ -52,9 +52,9 @@ class PGN(object): def from_value(pgn_value): logger.debug("PGN.@from_value, pgn_value=0x%08x" % (pgn_value)) pgn = PGN()

@@ -64,9 +64,9 @@ class PGN(object): canid = canid>>8 pgn = PGN()

milhead2 commented 1 year ago

Hi Brent,

Thanks you for working this out, we've been playing around the edges on this one.

Go ahead and push the changes,

Let me know if you have any changes

https://github.com/Trail-Tech/python-j1939

Miller


From: brearl @.> Sent: Wednesday, February 1, 2023 8:22 AM To: milhead2/python-j1939 @.> Cc: Subscribed @.***> Subject: [milhead2/python-j1939] 18 bit PGN changes break 16 bit masking (Issue #22)

External Email Think Before You Click.

The 18 bit PGN mask changes break the 16 bit masking and don't appear to work correctly for 18 bit PGNs. The patch below appears to correct this:

diff --git a/j1939/pgn.py b/j1939/pgn.py index 24611c1..4305ffc 100644 --- a/j1939/pgn.py +++ b/j1939/pgn.py @@ -42,9 +42,9 @@ class PGN(object):

@value.setter def value(self, value):

*

self.reserved_flag = (value & 0x080000) >> 17

*

self.data_page_flag = (value & 0x040000) >> 16

*

self.pdu_format = (value & 0x03FF00) >> 8

*

self.reserved_flag = (value & 0x020000) >> 17

*

self.data_page_flag = (value & 0x010000) >> 16

*

self.pdu_format = (value & 0x00FF00) >> 8 self.pdu_specific = value & 0x0000FF

MIL @.***, value=0x%08x, pdu_format=0x%08x" % (value, self.pdu_format))

@@ -52,9 +52,9 @@ class PGN(object): def from_value(pgn_value): @.***_value, pgn_value=0x%08x" % (pgn_value)) pgn = PGN()

*

pgn.reserved_flag = (pgn_value & 0x080000) >> 17

*

pgn.data_page_flag = (pgn_value & 0x040000) >> 16

*

pgn.pdu_format = (pgn_value & 0x03FF00) >> 8

*

pgn.reserved_flag = (pgn_value & 0x020000) >> 17

*

pgn.data_page_flag = (pgn_value & 0x010000) >> 16

*

pgn.pdu_format = (pgn_value & 0x00FF00) >> 8 pgn.pdu_specific = pgn_value & 0x0000FF return pgn

@@ -64,9 +64,9 @@ class PGN(object): canid = canid>>8 pgn = PGN()

*

pgn.reserved_flag = (canid & 0x080000) >> 17

*

pgn.data_page_flag = (canid & 0x040000) >> 16

*

pgn.pdu_format = (canid & 0x03FF00) >> 8

*

pgn.reserved_flag = (canid & 0x020000) >> 17

*

pgn.data_page_flag = (canid & 0x010000) >> 16

*

pgn.pdu_format = (canid & 0x00FF00) >> 8 pgn.pdu_specific = canid & 0x0000FF logger.info("{} staticmethod: PGN Creation, res={}, dp={}, pdu_format=0x{:02x}, pdu_specific=0x{:02x}".format(inspect.stack()[0][3], pgn.reserved_flag,

— Reply to this email directly, view it on GitHubhttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmilhead2%2Fpython-j1939%2Fissues%2F22&data=05%7C01%7Cmiller.lowe%40polaris.com%7C4047be2c2860433b30e908db04709608%7C85f78c4cad11473596240b2c11611dff%7C0%7C0%7C638108653819638803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=nYCC7MM6VIMsyXvQdVh5t88fiKvEAcLzshllXW0%2F%2Bc4%3D&reserved=0, or unsubscribehttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAEWRWUYTJEZVBS4LATMOE6TWVKEWHANCNFSM6AAAAAAUN52DIQ&data=05%7C01%7Cmiller.lowe%40polaris.com%7C4047be2c2860433b30e908db04709608%7C85f78c4cad11473596240b2c11611dff%7C0%7C0%7C638108653819638803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=a5V0jecywFLA6Fl7dRvYgKbSgpThgRRvl9HIyMwIYjo%3D&reserved=0. You are receiving this because you are subscribed to this thread.Message ID: @.***>

CONFIDENTIAL: The information contained in this email communication is confidential information intended only for the use of the addressee. Unauthorized use, disclosure or copying of this communication is strictly prohibited and may be unlawful. If you have received this communication in error, please notify us immediately by return email and destroy all copies of this communication, including all attachments.

brearl commented 1 year ago

Hey Miller,

I went ahead and sync'ed the trailtech fork with milhead2, did the work on that fork and then raised a PR.

I am unable to do anything more, the PR is here https://github.com/milhead2/python-j1939/pull/23 [https://opengraph.githubassets.com/bf7b14dd52fdabedf656133263871d3dccb80ee60e1dc5d5d1d74846de90cfc4/milhead2/python-j1939/pull/23]https://github.com/milhead2/python-j1939/pull/23 Fix 16 and 18 bit masking by brearl · Pull Request #23 · milhead2/python-j1939https://github.com/milhead2/python-j1939/pull/23 During python script testing, one of the commands "trap with timeout", that relies on mask patterns from python-j1939, stopped working. After a bit of investigation discovered that the changes to ... github.com


From: Miller Lowe @.> Sent: Wednesday, February 1, 2023 9:33 AM To: milhead2/python-j1939 @.> Cc: Brent Earl @.>; Author @.> Subject: Re: [milhead2/python-j1939] 18 bit PGN changes break 16 bit masking (Issue #22)

External Email Think Before You Click.

Hi Brent,

Thanks you for working this out, we've been playing around the edges on this one.

Go ahead and push the changes,

Let me know if you have any changes

https://github.com/Trail-Tech/python-j1939

Miller


From: brearl @.> Sent: Wednesday, February 1, 2023 8:22 AM To: milhead2/python-j1939 @.> Cc: Subscribed @.***> Subject: [milhead2/python-j1939] 18 bit PGN changes break 16 bit masking (Issue #22)

External Email Think Before You Click.

The 18 bit PGN mask changes break the 16 bit masking and don't appear to work correctly for 18 bit PGNs. The patch below appears to correct this:

diff --git a/j1939/pgn.py b/j1939/pgn.py index 24611c1..4305ffc 100644 --- a/j1939/pgn.py +++ b/j1939/pgn.py @@ -42,9 +42,9 @@ class PGN(object):

@value.setter def value(self, value):

*

self.reserved_flag = (value & 0x080000) >> 17

*

self.data_page_flag = (value & 0x040000) >> 16

*

self.pdu_format = (value & 0x03FF00) >> 8

*

self.reserved_flag = (value & 0x020000) >> 17

*

self.data_page_flag = (value & 0x010000) >> 16

*

self.pdu_format = (value & 0x00FF00) >> 8 self.pdu_specific = value & 0x0000FF

MIL @.***, value=0x%08x, pdu_format=0x%08x" % (value, self.pdu_format))

@@ -52,9 +52,9 @@ class PGN(object): def from_value(pgn_value): @.***_value, pgn_value=0x%08x" % (pgn_value)) pgn = PGN()

*

pgn.reserved_flag = (pgn_value & 0x080000) >> 17

*

pgn.data_page_flag = (pgn_value & 0x040000) >> 16

*

pgn.pdu_format = (pgn_value & 0x03FF00) >> 8

*

pgn.reserved_flag = (pgn_value & 0x020000) >> 17

*

pgn.data_page_flag = (pgn_value & 0x010000) >> 16

*

pgn.pdu_format = (pgn_value & 0x00FF00) >> 8 pgn.pdu_specific = pgn_value & 0x0000FF return pgn

@@ -64,9 +64,9 @@ class PGN(object): canid = canid>>8 pgn = PGN()

*

pgn.reserved_flag = (canid & 0x080000) >> 17

*

pgn.data_page_flag = (canid & 0x040000) >> 16

*

pgn.pdu_format = (canid & 0x03FF00) >> 8

*

pgn.reserved_flag = (canid & 0x020000) >> 17

*

pgn.data_page_flag = (canid & 0x010000) >> 16

*

pgn.pdu_format = (canid & 0x00FF00) >> 8 pgn.pdu_specific = canid & 0x0000FF logger.info("{} staticmethod: PGN Creation, res={}, dp={}, pdu_format=0x{:02x}, pdu_specific=0x{:02x}".format(inspect.stack()[0][3], pgn.reserved_flag,

— Reply to this email directly, view it on GitHubhttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmilhead2%2Fpython-j1939%2Fissues%2F22&data=05%7C01%7Cmiller.lowe%40polaris.com%7C4047be2c2860433b30e908db04709608%7C85f78c4cad11473596240b2c11611dff%7C0%7C0%7C638108653819638803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=nYCC7MM6VIMsyXvQdVh5t88fiKvEAcLzshllXW0%2F%2Bc4%3D&reserved=0, or unsubscribehttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAEWRWUYTJEZVBS4LATMOE6TWVKEWHANCNFSM6AAAAAAUN52DIQ&data=05%7C01%7Cmiller.lowe%40polaris.com%7C4047be2c2860433b30e908db04709608%7C85f78c4cad11473596240b2c11611dff%7C0%7C0%7C638108653819638803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=a5V0jecywFLA6Fl7dRvYgKbSgpThgRRvl9HIyMwIYjo%3D&reserved=0. You are receiving this because you are subscribed to this thread.Message ID: @.***>

CONFIDENTIAL: The information contained in this email communication is confidential information intended only for the use of the addressee. Unauthorized use, disclosure or copying of this communication is strictly prohibited and may be unlawful. If you have received this communication in error, please notify us immediately by return email and destroy all copies of this communication, including all attachments.

— Reply to this email directly, view it on GitHubhttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmilhead2%2Fpython-j1939%2Fissues%2F22%23issuecomment-1412356868&data=05%7C01%7Cbrent.earl%40polaris.com%7C5bdd9c675e764277acf308db04721b6b%7C85f78c4cad11473596240b2c11611dff%7C0%7C0%7C638108660357639199%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2BYUPGlkgjVSjR%2B2yrLEf%2BPcknf9KiL6xw%2Ff5g%2Fb1aJc%3D&reserved=0, or unsubscribehttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FASOUGLWQDVPE25HIGHUA7RTWVKF7DANCNFSM6AAAAAAUN52DIQ&data=05%7C01%7Cbrent.earl%40polaris.com%7C5bdd9c675e764277acf308db04721b6b%7C85f78c4cad11473596240b2c11611dff%7C0%7C0%7C638108660357639199%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Lky4OzDej8t4eAYOhrEwPkgMW2mb2cB35ni%2B1nbke70%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.***>

CONFIDENTIAL: The information contained in this email communication is confidential information intended only for the use of the addressee. Unauthorized use, disclosure or copying of this communication is strictly prohibited and may be unlawful. If you have received this communication in error, please notify us immediately by return email and destroy all copies of this communication, including all attachments.