[WIP] fix for #226

[ebroecker]

fix for missing "putSignalIntoFrame"

[codecov-io]

Codecov Report

Merging #247 into development will decrease coverage by 0.04%.
The diff coverage is 4.34%.

@@               Coverage Diff               @@
##           development     #247      +/-   ##
===============================================
- Coverage        28.77%   28.73%   -0.05%     
===============================================
  Files               29       29              
  Lines             6384     6394      +10     
  Branches          1616     1619       +3     
===============================================
  Hits              1837     1837              
- Misses            4391     4401      +10     
  Partials           156      156

Impacted Files	Coverage Δ
src/canmatrix/arxml.py	0.64% <0%> (-0.01%)	⬇️
src/canmatrix/canmatrix.py	68.37% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38ccd16...7b697da. Read the comment docs.

[altendky]

Mostly just a quick glance without knowing the bigger picture...

[altendky]

I would probably use a dict, though I'm not certain it is 'better'. Just keep it in mind.

[ebroecker]

good idea.

[altendky]

It has the cost of being less flexible for handling situations like if you only want some things to be case insensitive etc. But either way, it's small now and could be easily readjusted either direction in the future as needed.

[altendky]

Suggested change

	textValue = textValue.lower()
	textValue = textValue.casefold()

https://docs.python.org/3/library/stdtypes.html#str.casefold

Though I've heard debate on their example being proper...

[altendky]

I don't know where we are on this 'officially' but do we and lowerCamel or snake_case? I'll add this to the refactor ticket to consider.

#236 (comment)

[ebroecker]

... so you preferre snake_case . I tried to create new features in snake_case but this time, I missed my intent

[altendky]

Should I change the linked ticket to just state it will be 'snake_case'? I suppose that we could instead be picking an existing coding standard/checker (black, or something configurable) and that would cover many of these questions.

[altendky]

Suggested change

	returns a numerical value for common strings.
	returns a numerical string for common strings.

At least that's what it does presently.

[altendky]

@ebroecker, should I be a stickler for coverage? If so, go add tests... if not, merge away. :]

[ebroecker]

@ebroecker, should I be a stickler for coverage? If so, go add tests... if not, merge away. :]

@altendky
coverage for arxml is a hard job.
I'd have to create a 'dummy-arxml' first.
I'll do this in another PR

[altendky]

While we should have integration tests, it would be nice for more and more test coverage to come from tests closer to the unit level. For example, for this reason and others, guess_value() should be defined in global scope and can then be directly tested. Also, unless there's going to be a complete replacement of the parsing code (we at least vaguely discussed parsley or other parsing libraries) it should be broken down into smaller functions that can be called individually. For example, without having checked the details we ought to be able to call a function that parses a single option from a line in a sym file, and another function to parse a single line (perhaps a different function for lines in different places).

So, easy enough to get coverage on guess_value() here. Consider the couple tests and 'files' in canmatrix.tests.sym. My intent is that the file only contains the minimum necessary to execute the test. The point is to make it clear what is involved in the feature being tested and also that if we break something else, this test is less likely to fail.