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

Assertion checks, CI is back #51

Merged
merged 2 commits into from
Nov 2, 2018
Merged

Assertion checks, CI is back #51

merged 2 commits into from
Nov 2, 2018

Conversation

pavel-kirienko
Copy link
Member

image

PyDSDL version 0.1.0 is out: https://github.com/UAVCAN/pydsdl (also available on PyPI)
PyDSDL can do clever checks as proposed here by @kjetilkjeka (using exactly that syntax): https://forum.uavcan.org/t/new-dsdl-directive-for-simple-compile-time-checks/190/13 (not yet described in the specification, soon to be)

I fully comprehended the value of static checks only when I started using them myself here: they helped me to uncover a bit alignment issue in the PortID definition, which I promptly fixed. Luckily, this is a staging branch, so breaking changes are still possible, otherwise it would be a problem. Also, it is no longer necessary to go over each definition meticulously adding bits together to make sure everything is aligned properly, which is a really huge relief: you can just drop in @assert offset % 8 == {0}, and the parser (compiler) will statically prove that none of the possible valid combinations of preceding fields can produce an offset that is not byte-aligned.

For example, the set of bit length values for all valid encoded representations of uavcan.node.GetInfo.Response is: 320, 328, 336, 344, 352, 360, 368, 376, 384, 392, 400, 408, 416, 424, 432, 440, 448, 456, 464, 472, 480, 488, 496, 504, 512, 520, 528, 536, 544, 552, 560, 568, 576, 584, 592, 600, 608, 616, 624, 632, 640, 648, 656, 664, 672, 680, 688, 696, 704, 712, 720, 728, 736, 744, 752, 760, 768, 776, 784, 792, 800, 808, 816, 824, 832, 840, 848, 856, 864, 872, 880, 888, 896, 904, 912, 920, 928, 936, 944, 952, 960, 968, 976, 984, 992, 1000, 1008, 1016, 1024, 1032, 1040, 1048, 1056, 1064, 1072, 1080, 1088, 1096, 1104, 1112, 1120, 1128, 1136, 1144, 1152, 1160, 1168, 1176, 1184, 1192, 1200, 1208, 1216, 1224, 1232, 1240, 1248, 1256, 1264, 1272, 1280, 1288, 1296, 1304, 1312, 1320, 1328, 1336, 1344, 1352, 1360, 1368, 1376, 1384, 1392, 1400, 1408, 1416, 1424, 1432, 1440, 1448, 1456, 1464, 1472, 1480, 1488, 1496, 1504, 1512, 1520, 1528, 1536, 1544, 1552, 1560, 1568, 1576, 1584, 1592, 1600, 1608, 1616, 1624, 1632, 1640, 1648, 1656, 1664, 1672, 1680, 1688, 1696, 1704, 1712, 1720, 1728, 1736, 1744, 1752, 1760, 1768, 1776, 1784, 1792, 1800, 1808, 1816, 1824, 1832, 1840, 1848, 1856, 1864, 1872, 1880, 1888, 1896, 1904, 1912, 1920, 1928, 1936, 1944, 1952, 1960, 1968, 1976, 1984, 1992, 2000, 2008, 2016, 2024, 2032, 2040, 2048, 2056, 2064, 2072, 2080, 2088, 2096, 2104, 2112, 2120, 2128, 2136, 2144, 2152, 2160, 2168, 2176, 2184, 2192, 2200, 2208, 2216, 2224, 2232, 2240, 2248, 2256, 2264, 2272, 2280, 2288, 2296, 2304, 2312, 2320, 2328, 2336, 2344, 2352, 2360, 2368, 2376, 2384, 2392, 2400, 2408, 2416, 2424, 2432, 2440, 2448, 2456, 2464, 2472, 2480, 2488, 2496, 2504, 2512, 2520, 2528, 2536, 2544, 2552, 2560, 2568, 2576, 2584, 2592, 2600, 2608, 2616, 2624, 2632, 2640, 2648, 2656, 2664, 2672, 2680, 2688, 2696, 2704, 2712, 2720, 2728, 2736, 2744, 2752, 2760, 2768, 2776, 2784, 2792, 2800, 2808, 2816, 2824, 2832, 2840, 2848, 2856, 2864, 2872, 2880, 2888, 2896, 2904, 2912, 2920, 2928.

Copy link
Contributor

@kjetilkjeka kjetilkjeka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be exactly what we want, Awesome!

I would be OK with merging this even though we haven't fully defined how assert is going to work. This PR requires us to include the following in the DSDL specification:

  • Formalize some Integer type for const eval.
  • Formalize some Set of Integer type (or set of a generic type) for const eval.
  • Formalize some boolean type for const eval. ( since @assert <value> is probably going to accept a bool?)
  • Set equal operator that returns a bool.
  • Set max/min functions that returns the inner type of the set.
  • Elementwise multiply and remainder operations on sets.

I don't think there is something in the list over we're not going to end up wanting, so this is completely fine!


I think we should take this opportunity to formalize and unify other places where const evaluations is useful as well. I talked to @aasmune last week and got the impression that the following would be very useful for them as well. I will write about this on the forum: https://forum.uavcan.org/t/new-dsdl-directive-for-simple-compile-time-checks/190/14

@@ -28,7 +28,7 @@ Version.1 software_version
# For example, this field can be used for reporting the short git commit hash of the current
# software revision.
# Set to zero if not used.
truncated uint64 software_vcs_revision_id
uint64 software_vcs_revision_id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not truncated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Truncated makes sense only if narrowing conversions are expected. With 64-bit integer that would be redundant.

@pavel-kirienko pavel-kirienko merged commit a5d562c into uavcan-v1.0 Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants