PCMHammer P04

[Gampy]

Thank you for letting us know ... I'm sure it'll save someone else!

And just to make it clear for next readers ...

The OBDX Pro VT needs to be used in 1x mode only!

-Enjoy

[Tazzi]

Just made a pull request based off the develop branch that makes an edit to detect the OBDX Pro VT scantools and P04's to enforce 1x speed.
4x is still retained on all other ECUs as they are not affected.

[Gampy]

Reviewed and commented, please resolve.

Thank You

Edit;
On second thought, we need to think about this, at least I do ... No can do ATM, 6am meds messing with me brain!

-Enjoy

[Tazzi]

Have just revisited the code and made alterations plus a new pull.

[Gampy]

Things changed while I was commenting, you must still be making changes ... I have made several comments.

-Enjoy

[Tazzi]

I attempted a pull but gampy dismissed it due to a couple spaces (literal spaces).

My addition is in my OBDX branch. If its wanted to be merged in, someone is more then welcome to.

@Gampy, I am not having a personal dig at you, but if you are going to dismiss someones pull claiming they need to "adhere to that code bases guidelines", you should probably make sure the actual pcmhammer code adheres to this since there is over a dozen places where there is 'spaces' where there should be such as 'wait reader.ReadContents( cancellationTokenSource.Token);'

Or spaces with commas ect such as this: 'success &= ValidateRangeWordSum(type, 0x1E002, 0x1EFFF, 0x1E000 , "System");'

Topping that off is inconsistent uses of int, uint and uint32 throughout the app. THIS is something that should be maintained, but has not. But instead, 'spaces' is what dictates the addition of code.

When you decide to keep to your word of maintaining the same standard throughout the whole app, then I will update to remove spaces. Until then, your github suggestions are just wasting my time! (Again, not a dig at you, but your dismissing functionality over spaces that appear throughout the app).

This addition is not just for "my product", it fixes an issue with an ECU that can occur on many tools if they could even do 4x such as OBDLink or ELMs, which can include future tools or other J2534 tools that do not have a heavy enough pull down resistor on the line on bench setups. It is a load issue as per the SAE specifications for VPW which is not being met on a bench setup hence 1x speed is required or applying load (resistance) is required.

I do assume there will be a big response from Gampy about this, but Iv done my part, so I don't actually care about the response unless I get a PM indicating the code base is meeting the indicated code standard in all other places. Gampy, Iv marked this closed in OBDX todo list. Not my issue if you decide to dismiss, neither do I care to meet your designated standards (Not the apps clearly).

If it gets merged, great, if not.. Ill fork and leave a compiled version on OBDXs github. No skin off my back!

[antus]

Hey guys this is sad. It is true the whole codebase would benefit from improved consistency. I am getting worries that develop is over 100 commits from the last public release and there are several people willing to dabble and contribute but nobody has the time at the moment to put together the next release. It is true that "argument ,next argument" where the space is before the , is a pretty obvious fix to me. I saw there was a heap of posts last night but I was away from my PC and decided to let you sort it out. From what I saw the other day it was only a couple of lines of code. I guess we need to add that adding a linter and CI testing to the repo is going to be required to sort this out. But linting the code now would make the changes untrackable and develop branch has already diverged further than i'd like before the next release.

[In-Tech]

Oh the easier days of assembly

[antus]

I did a bit of looking around at C# linters as I think the best solution is really to install one of those. I found this 2 year old article here https://dev.to/srmagura/c-linting-and-f ... n-2021-bna that has a couple of fair reviews for a couple of different tools. I gave the solarlint tool a try and it was easy to add and use and found about 3800 warnings through the app. The autofix is nice, you can look at instance of a style rule being broken and then ask it to update the whole file or project. I just used the ruleset the review above is using, and I would change it a little if we used this tool. But I don't think now is the time to add the linter as we have diverged branches and it'll make re-merging hellishly difficult, but I would love to merge Gampys code as well the other branches back to develop, and then we could do this as well as get serious about some kind of release candidate and testing it. @Gampy I am happy enough that your asm kernels are good, and we can continue to test in develop after we get some more app code changes in as well. Would you consider sending a merge request?

[ProfessWRX]

Not sure what just happened but I've got a 3000lbs paperweight.

Am I right to think that there was no writing happening so I need to power cycle the PCM? It's beeping and flashing security at me.