Comments (9)
Very cool! I need to chew on some of these, but for the moment I'm having some challenges getting this integrated into the Makefile. What does the metrics
argument mean in there? The output I get right now is:
$ make analyze
cobra: cannot find 'metrics'
from sensor-watch.
Thanks so much, this is very helpful! It did expose several actionable things, both style and substance. One of the inconsistently checked return value warnings helped explain a glitch in the World Clock when exiting settings mode! And it also seemed wise to add comments to those empty loops to make it clear what they're for.
One other thing it exposed is some lazy coding on my part: the underlying IO calls for SERCOMs (SPI, I2C, UART) return status codes to indicate whether reads and writes were successful. Ideally, the watch API for this would return at least boolean values for success or failure, but as of now all my calls are void
(or in the case of watch_uart_getc
, the received character). This feels like a good place for future improvement, but it's not something I addressed in this round.
from sensor-watch.
Hi Joey, as far as suggestions would you want to perform any static analysis before this release?
from sensor-watch.
I would love to perform static analysis; I'm down with anything that improves the firmware's reliability and correctness. Do you have a particular tool that you would suggest using?
from sensor-watch.
I really like Cobra.
I use the built in rule sets, not the interactive portion (though that is cool too).
I'm not sure if you want to try to adhere to MISRA or anything, but the basic ruleset and cwe are pretty good. I can help configure it if you want to try it out!
I am not sure how this is usually done with Make, but I added a rule
COBRA = cobra -f
...
analyze:
@$(COBRA) metrics $(INCLUDES) $(DEFINES) $(SRCS)
and tested with it, but I am getting an error from Cobra that is going to be fixed with the next release.
from sensor-watch.
I got this working in the previous version of Cobra. Here's an attachment of the basic ruleset output.
You can find each rule's set of results by searching for the "===" indicator.
I'm not sure what is happening with the "bad field type 'fct'" lines.
Adding MISRA 2012 for fun.
from sensor-watch.
'metrics' is the rule file that Cobra is wanting to use. There are a few rulesets that come with Cobra. I ran 'basic' and 'misra2012'. Typically I run all of them but I didn't add all of that to the Makefile yet.
There is a configuration step in the Cobra docs that puts the path to the rulesets where Cobra can see them. Did you run that step?
It is on this page -- cobra -configure $COBRA/rules
from sensor-watch.
Ah, got it! I had put the binaries in kind of an odd spot so the rules directory was configured awkwardly. Also it looks like the bad field type 'fct'
lines aren't going to stdout (maybe to stderr?) so they're likely not related to the analysis.
There are definitely some actionable things in there, but I'm a bit confused by some of the other results; for example, I'm getting several of these:
=== Inconsistent checks of function return values:
1: _update checked 2 times out of 4
The thing is, _update is a void function and we never seem to check the return value at all. I can silence the warning by changing lines like
if ((date_time.unit.minute == 0) && (date_time.unit.second == 0)) _update(settings, state, state->offset);
to
if ((date_time.unit.minute == 0) && (date_time.unit.second == 0)) {
_update(settings, state, state->offset);
}
but I'm unclear as to whether what I was doing previously is actually incorrect, or if this is a false positive.
Similarly it's firing on some "empty while statements" like while(!SUPC->STATUS.bit.VREGRDY);
. I get this one: since the condition is just a bit in memory, static analysis sees it as a while(0) or while(1). But since the register's value will change when the underlying state of the hardware changes, this is actually correct.
Basically I'm wondering how I should interpret what it's telling me, whether the goal is to get to zero warnings, and if in cases where some of what's there is known to be correct, if there's a way to tell this to the tool so it can become part of the build steps and alert us when something is really off.
from sensor-watch.
Hey there. Sorry for the delayed reply.
Great point about the lines potentially going to stderr.
Yes, so I have used Cobra on a few projects, and it does generate some false positives, The void function one appears to be one.
For the while loops, it isn't saying that the value won't change, just that the while
doesn't contain any statements, like there are no curly braces that contain code to be executed as part of the while
. This could be an error, but in your case it is correct. I get it for the same reason though -- waiting on a register to change.
The goal is not to get zero warnings, and yes there are cases where it can be incorrect. I always check the results however to determine for myself if it is a false positive. So far there isn't a way that I know of to silence any particular warning though there is an effort to have Cobra support SARIF which would allow it to work with some static analysis tool sets.
Basically I like using Cobra since it is freely available and has caught many true issues when using CWE/MISRA/JPL/P10 rulesets. Though you have described some of the rough edges. I have a tool (that I unfortunately can't open source at the moment) that interprets Cobra logs and allows you do perform some filtering on them to make things a little easier in the meantime.
from sensor-watch.
Related Issues (20)
- adjusting the cadence in Nanosec seems to jumble the profile setting
- totp_face HOT 3
- TOTP no longer works as of 2 January 2024 HOT 5
- Tomato watchface corrupt display when timer expires while in background
- timer_face not working properly anymore HOT 2
- Tomato watch face not working anymore HOT 4
- Not entering LE mode under certain conditions HOT 3
- The LE mode is canceld when you swipe your finger across the display HOT 5
- Watch Freezes at Midnight in LE HOT 3
- Watch freezing at midnight in LE mode HOT 1
- Blank screen after update HOT 8
- Finetune face seems to have a mismatch between shown correction and stored correction HOT 4
- timer_face only shows 5 of 7 presets - sequel of issue #245 HOT 2
- timer_face sometimes no/aborted/quiet buzzing HOT 5
- place_face.c is missing HOT 2
- Lite board fails to boot if too many totp codes are configured. HOT 8
- Totp LFS face fails with certain codes and messes up the file system on physical watch HOT 4
- How to get serial output from USB? HOT 2
- Yellow LED constantly on for red board HOT 5
- Flashing firmware instructions for the command line HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sensor-watch.