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

asammdf v7.1.1 GUI: Various suggestions #743

Closed
MatinF opened this issue Aug 5, 2022 · 13 comments
Closed

asammdf v7.1.1 GUI: Various suggestions #743

MatinF opened this issue Aug 5, 2022 · 13 comments

Comments

@MatinF
Copy link
Contributor

MatinF commented Aug 5, 2022

Hi Daniel,

I've listed below a number of observations on v7.1.1 of the GUI (Windows 10, Python 3.8):

Export settings are too slim

Recommend expanding these slightly to avoid overflow:
image

Bus Logging uses LIN DBC as default

Recommend switching to CAN DBC as default, as that will be the use case for 99% of users
image

Graphical plots are not correctly "synced" to the data

In 7.0.7, adding a new plot would auto sync the plot around the data. Currently, the result is as below when initially plotting:

image

The interpretation of J1939 PGNs seems to differ in 7.1.0/7.1.1 vs. 7.0.7

I get 136 unique CAN IDs reported in the Bus Logging tab in 7.1.1 vs. 100 unique CAN IDs in 7.0.7. I've sent you a data sample and DBC.

@MatinF MatinF changed the title asammdf v7.1.1 GUI: asammdf v7.1.1 GUI: Various suggestions Aug 5, 2022
danielhrisca added a commit that referenced this issue Aug 5, 2022
@danielhrisca
Copy link
Owner

please try the development branch code

@MatinF
Copy link
Contributor Author

MatinF commented Aug 5, 2022

Thanks Daniel, that seems to fix most of these.

Remaining observations:

CAN ID difference

There is still the ID discrepancy between 7.0.7 and 7.1.1, I believe it's due to the fact that 7.0.7 filters out the CAN IDs that do not contain valid signals.

For example, in the sample data I've sent you, the CAN ID 0xCF00400 contains the valid EngineSpeed signal as you'll see if you plot it. In contrast, the CAN ID CF00421 contains purely 0xFF values in the payload and should be filtered out based on the default 'Ignore invalid signals' rule that is to be applied under J1939 (as is the case in v7.0.7).

image

CAN IDs shown when double clicking J1939 signal is the DBC ID, not "real" CAN ID

Due to the PGN matching logic of J1939, the CAN ID found in the DBC may not equal the actual CAN ID. In troubleshooting, it's useful to be able to "go back" into the raw bus logging file and look up a specific CAN ID based on a converted signal. Double clicking the signal allows a user to see the underlying CAN ID, but it's the CAN ID from the DBC file - not the actual CAN ID resulting in that signal.

As an example, EngineSpeed in the sample file I sent you shows the CAN ID from the DBC below. Ideally, it would have shown CAN ID CF00400 instead.

image

@MatinF
Copy link
Contributor Author

MatinF commented Aug 5, 2022

Another observation:

The 'crosshair' in plots is white, which is not ideal for white background plots

See below example:

image

@MatinF
Copy link
Contributor Author

MatinF commented Aug 9, 2022

One other comment: When displaying hex format CAN IDs in the tabular display, the default pre-padding is always as if the CAN ID is 29 bit. I suggest that when working with 11 bit CAN IDs, the padding changes accordingly. Alternatively, I sugges tentirely removing zero padding in front of the CAN IDs.

@danielhrisca
Copy link
Owner

Another observation:

The 'crosshair' in plots is white, which is not ideal for white background plots

See below example:

image

The cursor is now configurable form the menu (Settings -> Cursor)

@MatinF
Copy link
Contributor Author

MatinF commented Aug 9, 2022

Ok thanks Daniel, if possible it would be great if the default can be set to a color that can work both for black/white themes

@danielhrisca
Copy link
Owner

Ok thanks Daniel, if possible it would be great if the default can be set to a color that can work both for black/white themes

any sugegstions?

@MatinF
Copy link
Contributor Author

MatinF commented Aug 30, 2022

I suggest #e69138

@MatinF
Copy link
Contributor Author

MatinF commented Sep 5, 2022

The update to the crosshair colors solved that issue, thanks Daniel.

I think the main issue remaining are:

  1. J1939 'Ignore Invalid Signals' does not seem to work - invalid signals are not removed (7.2.0.dev2)

  2. Clicking a cell in the 'tabular display' causes asammdf to stall and crash on my end (7.2.0.dev2)

  3. Tabular display filters do not seem to work correctly, e.g. filtering 11-bit IDs to show all IDs above "700" does not work in 7.2.0.dev2 (but works in 7.1.1)

@MatinF
Copy link
Contributor Author

MatinF commented Sep 7, 2022

Quick update on 2. - it seems to be a dependency issue as our team member on another PC does not experience this. If you have any idea what dependency might be relevant here that would be great.

Below is my pip freeze:

aiobotocore==2.1.2
aiohttp==3.8.1
aioitertools==0.10.0
aiosignal==1.2.0
alabaster==0.7.12
asammdf @ git+https://github.com/danielhrisca/asammdf.git@64c22da25b56f73f881c9ed0725bd2e1384660c7
async-timeout==4.0.2
asynctest==0.13.0
attrs==21.4.0
Babel==2.10.3
bitstruct==8.13.0
black==22.6.0
boto3==1.24.55
botocore==1.23.24
can-decoder==0.1.3
canedge-browser==0.0.8
canmatrix==0.9.1
certifi==2021.10.8
charset-normalizer==2.0.12
click==8.1.2
colorama==0.4.4
docutils==0.17.1
fedex==2.4.1
Flask==2.1.2
Flask-Caching==1.10.1
frozenlist==1.3.0
fsspec==2022.2.0
future==0.18.2
idna==3.3
imagesize==1.4.1
importlib-metadata==4.11.3
influxdb-client==1.27.0
isal==1.0.1
itsdangerous==2.1.2
jellyfish==0.6.1
Jinja2==3.0.0
jmespath==0.10.0
livereload==2.6.3
lxml==4.9.1
lz4==4.0.2
MarkupSafe==2.1.1
mdf-iter==0.0.7
multidict==6.0.2
mypy-extensions==0.4.3
natsort==8.1.0
npm==0.1.1
numexpr==2.8.3
numpy==1.23.2
optional-django==0.1.0
packaging==21.3
pandas==1.4.3
pathlib2==2.3.7.post1
pathspec==0.9.0
platformdirs==2.5.2
psutil==5.9.1
pycountry==22.3.5
Pygments==2.12.0
pyparsing==3.0.9
PyPDF2==2.9.0
PyQt5==5.15.7
PyQt5-Qt5==5.15.2
PyQt5-sip==12.11.0
pyqtgraph==0.12.4
pyqtlet==0.3.3
pyqtlet2==0.8.0
PyQtWebEngine==5.15.6
PyQtWebEngine-Qt5==5.15.2
PySide6==6.2.0
PySide6-Addons==6.3.1
PySide6-Essentials==6.3.1
python-dateutil==2.8.2
pytz==2022.1
pyvat==1.3.15
QtPy==2.1.0
requests==2.28.1
Rx==3.2.0
s3fs==2022.2.0
s3transfer==0.6.0
scipy==1.6.1
shiboken6==6.2.0
six==1.16.0
snowballstemmer==2.2.0
Sphinx==1.8.5
sphinx-autobuild==2021.3.14
sphinx-rtd-theme==1.0.0
sphinxcontrib-serializinghtml==1.1.5
sphinxcontrib-websupport==1.2.4
suds==1.1.2
suds-jurko==0.6
tomli==2.0.1
tornado==6.2
typing-extensions==4.1.1
urllib3==1.26.9
us==2.0.2
waitress==2.1.1
Werkzeug==2.1.2
wrapt==1.14.0
yarl==1.7.2
zipp==3.8.0

@MatinF
Copy link
Contributor Author

MatinF commented Sep 7, 2022

Just an update on 2. above, I believe I've isolated the issue further as outlined here:
#751

@MatinF
Copy link
Contributor Author

MatinF commented Sep 15, 2022

Update:

I believe the filter issue has been resolved in the latest update. I'd still personally prefer that 11-bit IDs would not have the full 29-bit 0-padding in front, but that's more of a matter of taste than critical.

Since we've discussed the tabular cell issue separately, I believe we can 'pause' that for now and see if we may get some inputs from the pyqtlet2 team.

As such, only remaining item here is the J1939 invalid signals.

@MatinF
Copy link
Contributor Author

MatinF commented Sep 16, 2022

The J1939 invalid signals issue has been resolved in latest dev

@MatinF MatinF closed this as completed Sep 16, 2022
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

No branches or pull requests

2 participants